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

Add administrative brew locking function #3327

Merged

Conversation

G-Ambatte
Copy link
Collaborator

@G-Ambatte G-Ambatte commented Feb 25, 2024

This PR is work towards #3326.

This PR adds a check to the getBrew function that throws an error if the brew stub has a lock object with a state property set to 1 or 2.
State 1: locked, inaccessible.
State 2: locked, only accessible via Edit page.

The intended purpose of this function is to allow administrators to prevent access to brews that have identified issues - for example, DMCA take down requests.

@G-Ambatte G-Ambatte added the P3 - low priority Obscure tweak or fix for a single user label Feb 25, 2024
@G-Ambatte G-Ambatte self-assigned this Feb 25, 2024
Copy link
Member

@calculuschild calculuschild left a comment

Choose a reason for hiding this comment

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

This seems fine, and I would be ok merging this to form a basis for the next steps, since it shouldn't break anything. Do you want me to merge it now or keep tinkering on it?

@calculuschild calculuschild added the 🔍 R3 - Reviewed - Awaiting Fixes 🔧 PR is okayed but needs fixes before merging label Feb 27, 2024
@G-Ambatte
Copy link
Collaborator Author

This seems fine, and I would be ok merging this to form a basis for the next steps, since it shouldn't break anything. Do you want me to merge it now or keep tinkering on it?

I'll keep tinkering for now, I've used lock.state as an integer because I anticipated having multiple states (such as completely locked, or locked for editing only). If it will stay as simple as it is, then it may be more legible to future developers if I were to use named boolean states - such as locked and editable.

@G-Ambatte G-Ambatte temporarily deployed to homebrewery-pr-3327 February 28, 2024 02:55 Inactive
@G-Ambatte G-Ambatte temporarily deployed to homebrewery-pr-3327 February 28, 2024 03:03 Inactive
@5e-Cleric
Copy link
Member

Not sure if this is useful, but there is a 423 - Locked error code.

@G-Ambatte G-Ambatte temporarily deployed to homebrewery-pr-3327 March 17, 2024 05:41 Inactive
@G-Ambatte G-Ambatte temporarily deployed to homebrewery-pr-3327 March 17, 2024 08:42 Inactive
@G-Ambatte G-Ambatte added 🔍 R4 - Reviewed - Fixed! Awaiting final review⭐ PR review comments addressed and removed 🔍 R3 - Reviewed - Awaiting Fixes 🔧 PR is okayed but needs fixes before merging labels Mar 17, 2024
@G-Ambatte
Copy link
Collaborator Author

MongoDB entry:
image

Share Page:
image

Edit Page:
image

@G-Ambatte
Copy link
Collaborator Author

Includes a locked brew test:
image

@Gazook89
Copy link
Collaborator

I haven’t taken a strong look at this beyond just get comments/screenshots here, so pardon if I missed it:

i think this needs two things: a list of possible reasons the brew is locked (just a static thing, not anything actually related to a specific brew), and a path to contacting “administrators” like a link to the Reddit mod mail or whatever.

Right now it just looks overly harsh, with no indication of what the issue could be and no further help to get it fixed. Part of this is styling (big red letters) and part of it is content.

Further, it might just be best to say “unavailable” rather than locked, or even just use the same error page as the other “no brew” error. I’m not sure we need to tell all viewers that an author is getting DMCA’d (when we aren’t the ones really arbitrating/deciding that)

@calculuschild
Copy link
Member

Merging this as a first step toward #3326 . Thanks @G-Ambatte !

@calculuschild calculuschild marked this pull request as ready for review March 18, 2024 21:31
@calculuschild calculuschild temporarily deployed to homebrewery-pr-3327 March 18, 2024 21:32 Inactive
@calculuschild calculuschild merged commit dd6388b into naturalcrit:master Mar 18, 2024
2 checks passed
@G-Ambatte G-Ambatte mentioned this pull request Mar 19, 2024
4 tasks
@G-Ambatte G-Ambatte deleted the experimentalBrewLocking-#3326 branch March 27, 2024 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 - low priority Obscure tweak or fix for a single user 🔍 R4 - Reviewed - Fixed! Awaiting final review⭐ PR review comments addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants