Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add in persistentshare model and views and hook it into the target sh… #1138

Merged
merged 16 commits into from
Dec 17, 2024

Conversation

jnation3406
Copy link
Contributor

…are page.

There are so many urls added since I added both API views with django rest framework, and template / management views for normal tom users. The template views use the DRF views to actually create/delete persistentshare instances. There is also a version of URLS that applies to all targets, vs one filtered on a specific target for both creation and management. I tried to get permissions such that only a user with target_share permissions and permissions on that specific target should be able to create a persistent share on that target.

Screencast.from.2024-12-10.06-39-48.webm

Copy link
Contributor

@jchate6 jchate6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only part of the way through the review. Will try to finish tomorrow.

Copy link
Contributor

@jchate6 jchate6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some more things...

tom_dataproducts/data_processor.py Show resolved Hide resolved
tom_targets/base_models.py Outdated Show resolved Hide resolved
tom_targets/signals/handlers.py Outdated Show resolved Hide resolved
tom_targets/signals/handlers.py Show resolved Hide resolved
tom_targets/templates/tom_targets/target_share.html Outdated Show resolved Hide resolved
tom_targets/forms.py Outdated Show resolved Hide resolved
tom_targets/forms.py Outdated Show resolved Hide resolved
@jnation3406
Copy link
Contributor Author

I re-did the permissions and removed the share_target permission everywhere. Now its all using the change_target permissions + the persistentshare permissions. I also updated the pages to show/hide stuff based on your users permissions.

This is when you only have view_persistentshare permissions + target permissions
image

This is view + delete:
image

And this is view + delete + add (basically all permissions, change isn't used since they can't be modified):
image

@jchate6
Copy link
Contributor

jchate6 commented Dec 12, 2024

The permissions changes seem to work really well.
Will deleting the user who made a persistent share have any unforeseen repercussions? It doesn't look like there are any immediate problems, but I haven't tested actually triggering a share.

@jnation3406
Copy link
Contributor Author

The permissions changes seem to work really well. Will deleting the user who made a persistent share have any unforeseen repercussions? It doesn't look like there are any immediate problems, but I haven't tested actually triggering a share.

It shouldn't have any effect. Anyone with permissions can see/delete any persistent shares, the user that created it is just for knowledge purposes to know who you can ask for why they did that in your collaboration.

@jnation3406 jnation3406 requested a review from jchate6 December 13, 2024 08:29
@jchate6
Copy link
Contributor

jchate6 commented Dec 13, 2024

I get an error when I try to add a persistent share via the admin interface:
TypeError: BaseModelForm.__init__() got an unexpected keyword argument 'user'

@jnation3406
Copy link
Contributor Author

I get an error when I try to add a persistent share via the admin interface: TypeError: BaseModelForm.__init__() got an unexpected keyword argument 'user'

Oops I had broken that when changing the forms before. It's fixed now.

Copy link
Contributor

@jchate6 jchate6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@jchate6 jchate6 merged commit 820fa5d into dev Dec 17, 2024
24 checks passed
@jchate6 jchate6 deleted the feature/persistent_sharing branch December 17, 2024 01:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Merged (to dev)
Development

Successfully merging this pull request may close these issues.

2 participants