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 details to pub list, and fix perms #512

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

Mark-Powers
Copy link
Contributor

Adds review information to publications in the list view. Also now shows publications from all of a user's projects, not just those they have submitted (which was confusing with managers). Also fixes a delete permission issue: we were not checking ownership at all, and this ensures only the owner can delete what they have submitted.

image

@@ -69,16 +68,25 @@ def user_publications(request):
try:
del_pub_id = request.POST["pub_ref"]
logger.debug("deleting publication with id {}".format(del_pub_id))
Publication.objects.get(pk=del_pub_id).delete_pub()
pub = Publication.objects.get(pk=del_pub_id)
if pub.added_by_username != request.user.username:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the permissions fix change

except Exception:
logger.exception("Failed removing publication")
messages.error(
request,
"An unexpected error occurred while attempting "
"to remove this publication. Please try again",
)
mapper = ProjectAllocationMapper(request)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This section now queries for publications for all of the user projects, not just publications the user has submitted.

Copy link
Contributor

@msherman64 msherman64 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 to me, thanks for calling out the permissions change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants