BDMS-402: data visibility/review feature review #312
Conversation
… business logic - rewrote tests/features/data-visibility-and-review.feature scenarios to match the business logic covered in release_status.feature while preserving the feature’s description and rules - expanded public and staff access coverage so visibility and review_status behavior mirrors legacy workflows - documented workflow, management, integrity, and special cases to clearly separate visibility and review in business logic
chasetmartin
requested review from
jacob-a-brown,
jirhiker,
kbighorse,
ksmuczynski and
marissafichera
jacob-a-brown
left a comment
jacob-a-brown
left a comment
There was a problem hiding this comment.
Overall I think it looks good! And describes these situations well. I think that some of the steps could be made a bit more clear, such as indicating that the visibility field is set to "internal" or "public."
I often made a comment in the first appearances of these situations and if you agree with my feedback then they should be applied elsewhere too.
|
|
||
| @management @status_change | ||
| Scenario: Making internal data public without re-review | ||
| Given a <data_type> is currently visibility internal and review_status approved |
There was a problem hiding this comment.
@jacob-a-brown let me know if this makes more sense with the system?
There was a problem hiding this comment.
I think this looks good! Will the staff member already have the permissions, or will be obtained for the actions? If they already possess the permissions then I think it could read And staff has the correct permissions to change visibility, or something to that effect. If they need to be given the permissions then I think it looks good as-is.
jacob-a-brown
left a comment
jacob-a-brown
left a comment
There was a problem hiding this comment.
I think it looks quite good! Just a few minor comments.
|
|
||
| @management @status_change | ||
| Scenario: Making internal data public without re-review | ||
| Given a <data_type> is currently visibility internal and review_status approved |
There was a problem hiding this comment.
I think this looks good! Will the staff member already have the permissions, or will be obtained for the actions? If they already possess the permissions then I think it could read And staff has the correct permissions to change visibility, or something to that effect. If they need to be given the permissions then I think it looks good as-is.
| @staff_access @visibility | ||
| Scenario Outline: Staff can access all data and its review status | ||
| Given I am an authenticated staff member | ||
| And I have permission to view internal data |
There was a problem hiding this comment.
We will need to be more granular than this. E.g only amp users can few amp data, etc.
|
I think if the goal of this PR is to make Once you merge this, you can continue negotiating the resultant |
Why
This PR addresses the following problem / context:
data-visibility-and-review.featuregiven the now documented legacy business logic inrelease_status.featureHow
Implementation summary - the following was changed / added / removed:
data-visibility-and-review.featurescenarios to match the business logic covered inrelease_status.featurewhile preserving the new feature’s description and rulesrelease_statusNotes
Any special considerations, workarounds, or follow-up work to note?
- This PR is just meant as a discussion point - Should we change the proposed feature like this to ensure that business logic currently documented in the
release_status.featureis properly covered by thedata-visibility-and-reviewproposal?