Filter out audit logs that a user cannot see #570
Conversation
bloodearnest
force-pushed
the
audit-visibility
branch
from
7cc5fe1
to
ebd5abf
Compare
There was a problem hiding this comment.
I realise that some of the questions are about previous comment-visibility functionality that isn't specific to the audit log, but given that this is refactoring the previous visibility code, I think it's probably worth fixing here.
Some notes in pictures:
-
Here an author (researcher_1) has made a public comment before submitting the request. It's visible to everyone, but to the output-checkers it says "only visible to you until reviews are complete"
-
Here 2 output-checkers have completed their review, but haven't returned/rejected yet. Status is REVIEWED, so the public comment from output-checker 1 is NOT visible to the researcher yet, but it says it's visible to all.
-
Just to confirm - this is researcher 1's view. They can only see their initial, pre-submission comment.
-
This is the author's view after an output-checker rejected the request. The turn has not incremented, and the author doesn't see any of the public comments that were made before rejection. I think this is incorrect - otherwise there's no notification to the researcher on the reason for the rejection (presumably less important when it moves to released, but for withdrawn, the user could make a comment to say why they withdrew it, which would never be exposed to anyone else)
Right, there is separate ticket for this, think #512 For the first 3 screen shots, I think the actual visiblity is correct (i.e. whether its shown or not)? But the text explaining the visibility is all kinds of wrong, which the issue above is tracking. For the 4th screenshot, that is a bug, as noted in my reply to another comment: #570 (comment) |
bloodearnest
force-pushed
the
audit-visibility
branch
2 times, most recently
from
cb67667
to
eb60083
Compare
|
I have renamed and clarified things with comments as suggested. I have added coverage and fixed the interaction between reject/approved/release and visible comments by taking COMPLETE status into account. I have also fixed #512 I think by only blinding comments from the current round. |
This will allow us to use the exact same logic for filtering the audit log for visibility as well.
It's doesn't provide much value, and the logic of filtering for visibility when we may have multiple active requests is quite complex. We can revisit this later, perhaps.
This will bypass visibilty filtering, which is desirable in tests, as we mostly want to check that our audits were persisted. It allow use to repurpose the BLL get_audit_log message return logs filtered for visibility, which is the normal production use case.
This reflects the changes made in earlier commits, and that we are only using this BLL method for request/group pages.
We make AuditEvent satisfy use the VisibleItem protocol, so we can use `filter_visible_items` to filter the audit log appropriately. We record the review_turn an AuditEvent is generated in to facilitate this. Exposed we'd couple in technically incorrectly constructed AuditEvents in tests, which were fixed.
This meant extending the big multi-round walk through test to assert audit log visibilties. This ended up adding a supportting VisibleItems helper to keep the many many assertions readable. The large test is even bigger now, but given the complexity of setting up the state, I think this is still the best way to test this.
Add more comments, and rename can_see_private to user_can_review
Now its being used in the generic filter_visible_items logic, it applies to audit logs as well as comments
Previously, our test case didn't get the request state over the line to where the phase was COMPLETE, so we missed the fact that final comments might not be shown. So I changed the big test to reject instead of return, which moves us to COMPLETE phase, and then added the logic handling correctly to filter_visible_items.
Previously, whilst in INDEPENDANT phase, all comments would be shown with the independent description, for researchers or output checkers Now, that logic only applies to comments from the current round, which are the only ones subject to blinding. Additionally, I noticed that blinded comment text didn't make it clear whether it was a public or private comment, so it now builds on that information. Added an explicit test to cover this logic. Fixes #512
The language was too long to fit inside the pill. Given these comment visibilites are only shown to output-checkers, we just used the "Private", or "Public", and an optional "Blinded" one. I tried to add tooltips for these with the text description, but they don't current work properly inside pill components. Thomas is looking at tooltips, so we can fix that then. I've left the machinery for displaying them intact, but have commented out in the template. I'll add a ticket to renable the tooltips once they work inside a pill.
bloodearnest
force-pushed
the
audit-visibility
branch
from
eb60083
to
03818fd
Compare
Co-authored-by: Becky Smith <becky.smith@thedatalab.org>
This change ensures that users don't see audit messages that they shouldn't see.
It uses exactly same logic as previous work hiding comments/votes from users. So, this means
Fixes #534