-
Notifications
You must be signed in to change notification settings - Fork 39
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 seperate dashboard and submissiondetail views for partner and com… #1153
Conversation
abe7fb1
to
5dac0ed
Compare
…munity reviewer roles.
5dac0ed
to
a5323ba
Compare
…ew, edit. Minor refactoring.
@@ -36,6 +36,39 @@ <h1 class="gamma heading heading--no-margin heading--bold">Dashboard</h1> | |||
|
|||
{% endif %} | |||
</div> | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a "if my_submissions" check to only show this if the reviewer has submissions, I believe most will not have any.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, updated.
…nd status of submission in edit/review phase
…munity review stage
reviewer_view = ReviewerDeterminationDetailView | ||
partner_view = ApplicantDeterminationDetailView | ||
community_view = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@frjo Setting it to None will create a bug which will not allow community reviewers to see the determination for submissions submitted by them. What is the requirement for visibility of determination when the user is a community reviewer or a partner?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Parbhat community reviewers as well as partners can reuse the ReviewerDeterminationDetailView.
Everyone that can view a submission should also be able to view its determinations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@frjo Thanks for the information. We have to create new views for partners, community reviewers as these need different checks.
opentech/apply/funds/views.py
Outdated
if submission.user == request.user: | ||
return ApplicantSubmissionDetailView.as_view()(request, *args, **kwargs) | ||
# Only allow reviewers in the submission they are added as reviewers or has reviewed earlier | ||
reviewer_has_access = submission.reviewers.filter(pk=request.user.pk).exists() or submission.reviews.values('author').filter(author=request.user.pk) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New take on this from OTF. Reviewers should now be able to view any submission that have reached an "AC review" state or higher and have not been dismissed.
Can we make use of "permissions.can_view()" for this? Or should we add something similar to COMMUNITY_REVIEW_PHASES where we list all the states reviewers should have access to, i.e. REVIEW_VIEW_ACCESS_PHASES?
Other better solutions?
(Reviewers should, as it is now and have been for a long time, only be able to review submission they have been assigned to.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this needs some thinking we can revert to the current functionality where reviewers can see all submission. It has been like that from day one so no harm in postponing updating this a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting the other nice improvements in this PR out is more important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the information. We can use permissions.can_view() along with other required checks. It seems better to handle it in another PR. We also need to change ReviewerDeterminationDetailView
as currently, a reviewer can see any determination as well.
…iewers visibility
The dashboard and submission detail view are visible for Partner and Community Reviewer during the test. Thanks! I assigned this application https://test-apply.opentech.fund/apply/submissions/24/ to 1) Partner and 2) both "Test Reviewer" and "Maya Reviewer". The application appears in the Partner's dashboard, but NOT for community reviewers. See image below of Test Reviewer's dashboard without application: |
@fourthletter I can not access the test URL but an application should be in the community review stage to appear in the dashboard of community reviewer. |
…munity reviewer roles.
Fixes #1147
Fixes https://github.com/OpenTechFund/issues.otf/issues/5