Skip to content

Conversation

@benjaminkreen
Copy link
Contributor

@benjaminkreen benjaminkreen commented May 9, 2018

JIRA issue: https://jira.plos.org/jira/browse/APERTA-12923

What this PR does:

Just a little fine tuning of our new awesome permissions system :D

Special instructions for Review or PO:

As a reviewer you should now see the links to uploaded files. This may also fix https://jira.plos.org/jira/browse/APERTA-12923?focusedCommentId=214306&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-214306 though i'm still mostly adhering to the AC here


Code Review Tasks:

Author tasks (delete tasks that don't apply to your PR, this list should be finished before code review):

  • I have ensured that the Heroku Review App has successfully deployed and is ready for PO UAT.
  • I added an entry to ## [Unreleased] in CHANGELOG.md

Reviewer tasks (these should be checked or somehow noted before passing on to PO):

  • I read through the JIRA ticket's AC before doing the rest of the review
  • I ran the code (in the review environment or locally). I agree the running code fulfills the Acceptance Criteria as stated on the JIRA ticket
  • I read the code; it looks good
  • I have found the tests to be sufficient for both positive and negative test cases

end

delegate_view_permission_to :decision
delegate_view_permission_to :revise_task
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm unsure when the revise task springs into existence, do you all know?

Copy link
Contributor

Choose a reason for hiding this comment

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

When the paper is sent back to the author.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so maybe it should be

def revise_task_or_decision
  revise_task || decision
end

delegate_view_permission_to :revise_task_or_decision

but then i feel like i'm overloading our system? or is this fine?

Copy link
Contributor

Choose a reason for hiding this comment

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

Decisions are not first class objects for use in the roles and permissions system. That is, it never makes sense to check can? :view, decision.

I think checking if the user can view the revise task makes sense, if that is where they upload decision attachments.

Choose a reason for hiding this comment

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

There is more than just decision attachments...
How about cover letter?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I plan on reviewing attachment permissions per role for:
Response to reviewers
Cover Letter
Reporting Guidelines
Reviewer report
Invite reviewers
invite academic editor
upload manuscript
reporting guidelines
Figures and
Supporting Info

@egh
Copy link
Contributor

egh commented May 9, 2018

This looks correct to me, but I haven't tested it.

@sabajovic
Copy link

Can you both test this? Before pushing further?

@egh
Copy link
Contributor

egh commented May 9, 2018

Confirmed that this works on https://plos-ciagent-pr-4077.herokuapp.com/papers/yetijour.1000011

Logged in as an invited reviewer, I can download the attachment uploaded to the "Response to Reviewers" task.

@egh
Copy link
Contributor

egh commented May 9, 2018

Also confirmed that an admin user can replace and delete the attachment.

@benjaminkreen benjaminkreen merged commit 1ee439f into master May 9, 2018
@benjaminkreen benjaminkreen deleted the bugs/APERTA-12923-change-view-permission-delegation branch May 9, 2018 19:08
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.

6 participants