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

fix accessibility of silenced user posts #384

Merged

Conversation

@colin-axner
Copy link
Contributor

@colin-axner colin-axner commented Sep 17, 2020

Change view post collection queries to verify that the authenticated user viewing a silenced collection is either the owner or admin. Previous behaviour was only checking if the post owner was also the collection owner which always returned true, causing the if statement to be skipped.

closes: #374


  • I have signed the CLA
Change view post collection queries to verify that the authenticated user of a silenced collection is either the owner or admin
@mrvdb
Copy link
Collaborator

@mrvdb mrvdb commented Oct 1, 2020

I've applied this patch for qua.name. Works for me with one exception, which may be a local issue, I haven't dived in yet.

Posts from silenced users are still visible in the 'reader' ( URI '/read' ) After restarting writefreely they are not visible anymore. I think posts from silenced users should also be removed from the 'reader' page without having to restart writefreely (possibly a caching issue?)

@mrvdb mrvdb requested a review from thebaer Oct 1, 2020
@thebaer
Copy link
Contributor

@thebaer thebaer commented Oct 1, 2020

I agree, we should make it so silenced users are immediately removed from the Reader, too. This does sound like a caching issue -- maybe we could invalidate the cache anytime the Reader is enabled and a new user is silenced? Probably around here:

https://github.com/writeas/writefreely/blob/f534ee1deccbe5d161349554efa9aa0a2aac55df/read.go#L159-L161

@colin-axner
Copy link
Contributor Author

@colin-axner colin-axner commented Oct 9, 2020

Sounds good. Would it be safe to set tl.posts = nil before calling updateTimelineCache to force the cache to be reset or alternatively pass in a boolean indicating a cache override?

@thebaer
Copy link
Contributor

@thebaer thebaer commented Oct 13, 2020

I think the best way would be to pass a boolean that invalidates the cache. Since this is an expensive query, we prefer to keep tl.posts permanently populated after it first loads -- even if it serves stale content to a visitor or two, it keeps the Reader section responsive.

@colin-axner
Copy link
Contributor Author

@colin-axner colin-axner commented Oct 26, 2020

maybe I am missing something, but wouldn't Memo.Invalidate need to be modified to support this functionality.

The cache is an unexported field so in order to forcibly reset the cache, Invalidate needs to take in a boolean to bypass the last duration.

Should I instead add a third function Reset to forcibly reset the cache without breaking API?

@thebaer
Copy link
Contributor

@thebaer thebaer commented Nov 11, 2020

Should I instead add a third function Reset to forcibly reset the cache without breaking API?

Yes, I think that'd be the best route to go.

@colin-axner colin-axner marked this pull request as draft Dec 6, 2020
colin-axner added 2 commits Dec 6, 2020
Modify updateTimelineCache to allow a boolean to indicate that the cache should be forcibly reset
@colin-axner
Copy link
Contributor Author

@colin-axner colin-axner commented Dec 6, 2020

made the changes based on the linked pr. I'm not sure if I reset the cache in the right place after silencing the user? It is unclear to me if there's a different handler called when an admin clicks "silence" on a user.

Still need to manually test either via modifying the go.mod locally or merging the web-core pr and updating the commit. Leaving as a draft until the web-core pr is merged

Add back else clause after realizing the error check doesn't return after logging.
@thebaer thebaer marked this pull request as ready for review Mar 6, 2021
@thebaer
Copy link
Contributor

@thebaer thebaer commented Mar 6, 2021

Moving this out of draft, now that writeas/web-core#11 is merged.

Update go.mod to use latest commit on web-core
Copy link
Contributor Author

@colin-axner colin-axner left a comment

updated the web-core commit. Will try to test this week

go.sum Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
read.go Outdated Show resolved Hide resolved
@colin-axner
Copy link
Contributor Author

@colin-axner colin-axner commented Apr 7, 2021

I have tested these changes and they work as expected. When the user is silenced, the posts are not viewable directly or on the reader. When the user is unsilenced, the posts will be added back to the reader once the cache is refreshed

The UI is looking better. Looking forward to the next release

@thebaer
thebaer approved these changes Apr 7, 2021
Copy link
Contributor

@thebaer thebaer left a comment

Yep, tested and this works great! Thanks!

@thebaer thebaer added this to the 0.13 milestone Apr 7, 2021
@thebaer thebaer merged commit ac7583e into writefreely:develop Apr 7, 2021
@colin-axner colin-axner deleted the colin-axner:374-fix-silenced-post-accessibility branch Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

3 participants