Add blocking notification to EditPage

[G-Ambatte]

This PR contributes towards Issue #3326.

This PR adds a notification to the EditPage which must be acknowledged before the brew itself will become editable.

EditPage:

MongoDB entry:

[ericscheid]

Some thoughts:

1. This covers the case of initially loading /edit/:id .. but what happens when an author has opened a non-locked brew, but that brew has since had the lock applied?

This can happen as authors do have a habit of opening a brew to edit and leaving it open in their browser for long periods of time (e.g. weeks).

2. Does this click-to-acknowledge persist? That is, having obtained edit access for this session, if they close the brew and later go to edit it again will they see this message again? Or does click-to-acknowledge update the mongo record for the brew (e.g. editor-ack: <some-date> .. I would recommend a date rather than simply true, btw).

[calculuschild]

1. This covers the case of initially loading /edit/:id .. but what happens when an author has opened a non-locked brew, but that brew has since had the lock applied?

I don't think we have any way to push notifications out like that without creating a live socket connection for every editing session, which adds some complexity. Something like that would be a whole separate project, but eventually that might be something we add.

2. Does this click-to-acknowledge persist? That is, having obtained edit access for this session, if they close the brew and later go to edit it again will they see this message again?

I am imagining whatever notice we send should reappear every time the edit page is re-opened. If they are actively in violation of something I would rather keep reminding them than let them mute it. That said, I'm not really sure how lax or strict we should be in this type of situation.

[5e-Cleric]

That said, I'm not really sure how lax or strict we should be in this type of situation.

I think the question here is what is the locking function here to prevent? what type of content are we expecting to lock? confidential doc leaks? an alien cult manifesto? communist propaganda? (its a joke but you get what i mean).

It is a locking function, for content the user can rewrite or already has in their system. If the content of the brew is so bad it is deserving to lock the document, why not delete it entirely?

I understand the idea behind the locking, not so sure of its usefulness. Better to have it and not use it than need it and not have it, for sure.

[ericscheid]

I don't think we have any way to push notifications out like that without creating a live socket connection for every editing session, which adds some complexity. Something like that would be a whole separate project, but eventually that might be something we add.

We could throw the notification upon the first (auto)save of that brew, based on info received then.

[calculuschild]

That's a good point. Autosave creates regular periodic requests which could be used this way.

[ericscheid]

It is a locking function, for content the user can rewrite or already has in their system. If the content of the brew is so bad it is deserving to lock the document, why not delete it entirely?

There might be a copyright claim against an image in a brew. Deleting the entire brew would be an overreaction.

[5e-Cleric]

It is a locking function, for content the user can rewrite or already has in their system. If the content of the brew is so bad it is deserving to lock the document, why not delete it entirely?

There might be a copyright claim against an image in a brew. Deleting the entire brew would be an overreaction.

locking the brew for a copyright claim seems too much for me, however, the "get that image out or we will lock it" would seem more appropiate to me. IDK

[calculuschild]

It is a locking function, for content the user can rewrite or already has in their system. If the content of the brew is so bad it is deserving to lock the document, why not delete it entirely?

There might be a copyright claim against an image in a brew. Deleting the entire brew would be an overreaction.

locking the brew for a copyright claim seems too much for me, however, the "get that image out or we will lock it" would seem more appropiate to me. IDK

In response to a DMCA notice, we are required by law to remove the offending content from public access until the issue is resolved. We as the hosting service are not liable for our users' copyright infringement unless we knowingly allow infringing content to remain accessible after receiving a notice.

[G-Ambatte]

1. Seeing this popup in action, it occurs to me that it behaves as kind of a mini error page. Would it make more sense to use the common "Error Page" for this message? And put the "click to unlock" button on there?

Rather than creating a new LockNotification component, we could instead throw, and include in that error some function to disable the lock temporarily.

2. The error should probably explain what being "locked" means. I.e. the brew is not visible via the share page, etc.

Currently the only fixed text is "BREW LOCKED", everything else comes from the lock.message field. It should be trivial to add some explanatory text.

3. So "Click to unlock" just takes you to the editor as normal, but doesn't actually lift the lock. Is there a better name for that button, like "CLICK TO CONTINUE TO THE EDITOR".

Should be a fairly simple change.

4. I think a better use for this kind of "popup" thing might be for a "submit to admins" thing. I'm picturing this:

* User tries to open their edit page, and hits the error page instead. This is where all the details are. "your brew is locked. Here are all the details, and what this means for you. etc. Click here to continue to the editor"

* User enters the editor, where this popup now appears with a smaller message: "Your brew is locked. (locking message). Once you have resolved this issue, click here to notify the administrators for review. (BUTTON "REQUEST UNLOCK"). Click (OTHER BUTTON) to temporarily hide this notification (will reappear when the page is next reloaded).

A method for requesting locks be removed will need to be created, which is intended to be added in a later PR, once functionality to actually add/remove/otherwise update locks has been created.

5. It might make sense to reuse (and maybe update) the "notificationPopup.jsx" component we already have instead of making a separate component element.

As NotificationPopup is a non-blocking and permanently clearable message, it has a different purpose to this notification; although I suspect it might work with some modifications.

[G-Ambatte]

"This is a test message from the database." is the lock.message property. All other text in the notification is currently fixed text and will appear in every lock notification.

The "REQUEST LOCK REMOVAL" button is currently attached to a stub function.

[G-Ambatte]

It occurs to me that we may want two messages - one for the ErrorPage, and one for the LockNotification. The general public needs only to know that the brew is locked, while the author needs to know the specifics of why it is locked... As such, an editMessage and a shareMessage might be appropriate.

[G-Ambatte]

Fixed the typo in the NOT YET IMPLEMENTED message:

[G-Ambatte]

Current state of this PR:

Attempting to access a locked brew by any method other than edit results in an error. The error code (1001) and message (THIS IS THE TEST SHAREMESSAGE) are set on the lock object. Internally,the error uses a HBErrorCode value of 100.

Attempting to access a locked brew via edit results in the LOCKED notification. The message (This is the test EditMessage.) is set on the lock object. It is a different message to that displayed on the Error page.

Clicking CONTINUE TO EDITOR bypasses the lock notification and allows access to the Editor. The notification will return the next time the getBrew function is called - in this usage scenario, typically on the next page refresh. Clicking REQUEST LOCK REMOVAL currently pops a NOT YET IMPLEMENTED message box, which reports the ShareID of the locked brew. It is intended to pass this ID to the /api/lock/review/request/:id route from PR #3421 once that PR is finished and merged.

The lock object on the MongoDB entry is as follows:

[dbolack-ab]

The lock object on the MongoDB entry is as follows:

Just one question. is there a scenario where these need a custom, per-lock message?

[5e-Cleric]

Just one question. is there a scenario where these need a custom, per-lock message?

Locking will be manual either way, might as well personalize messages to the specific reason of the locking. Also way easier to tell the user what is exactly wrong rather than having them guess.

[Gazook89]

I think custom messages are a good idea, for the same reason as @5e-Cleric --- rather than them coming to reddit to ask why their brew is unlocked, we can anticipate the question and put the answer right in the error screen especially since we don't expect this to be super common (hopefully).

Also, we could have perhaps a code that corresponds to the type of lock, if we ever expect to lock things for reasons beyond DMCA. I assume the 1001 error code is just a generic code, so this would be in addition to that (or appended to that as 1001-1 1001-2 etc). If we ever wanted to find out the quantity of types of locks, this would make it easier. But perhaps that is just not worthwhile for a tool such as this.

[5e-Cleric]

I assume the 1001 error code is just a generic code, so this would be in addition to that (or appended to that as 1001-1 1001-2 etc).

I believe there are plenty unused http error codes for us to use some, 452 - 499 are unused, so we could create some, here some suggestions:

HTTP Code	Suggested Use
455	Generic Lock
456	Copyright issues
457	Confidential Information Leakage
458	Sensitive Personal Information
459	Defamation or Libel
460	Hate Speech or Discrimination
461	Illegal Activities
462	Malware or Phishing
463	Plagiarism
465	Misrepresentation
466	Inappropriate Content

As written in RFC 9110 Section 15.5 about error codes 4xx:

Except when responding to a HEAD request, the server SHOULD send a representation containing an explanation of the error situation, and whether it is a temporary or permanent condition. These status codes are applicable to any request method. User agents SHOULD display any included representation to the user.

[G-Ambatte]

Currently, the message and code can be set to anything - I used 1001 in my test lock simply because it doesn't get used anywhere else, so I could be sure it was coming from my code. I anticipate that we will be able to build in some suggested entries at the UI stage.

[5e-Cleric]

As a template:

Dear User,

We regret to inform you that your document titled [Document Title] has been locked and is no longer accessible due to a violation of our content guidelines. This action was necessary because the document Violates copyright laws || Contains offensive content || Includes illegal material.

We take these matters very seriously to ensure the safety and integrity of our platform for all users.

We ask you to remove any offending content, once you have, you can submit a request for us to remove the lock; once removed, users will be able to access the share page of this brew once again.

If you believe this action was taken in error, or if you have any questions or concerns, please contact our team at reddit for further assistance.

Thank you for your understanding and cooperation.

Sincerely, NaturalCrit

We should also specify what is the offending content as well as what is the specific issue.

[G-Ambatte]

We should also specify what is the offending content as well as what is the specific issue.

IMHO there is no point to the notification message if it doesn't identify what needs to changed.

---

It is my intention that any template message should still be fully editable by the person applying the lock; most likely, selecting a template would only populate the field and still require the case-specific details be entered manually.

I would tend towards a shorter template message, something more like this:

This document has been locked and is no longer accessible due to reason. Before it can be unlocked, the following issues must be corrected:

• specific issue

Once you have corrected the above issue(s), click "REQUEST REVIEW". Once an Administrator confirms that all issues have been corrected, the lock will be removed.

If you have any questions about the process, you may contact our team via Reddit.

Sincerely, Homebrewery Admin Team

[calculuschild]

@G-Ambatte Is this one ready for another review pass or are you still working on it?

[G-Ambatte]

@G-Ambatte Is this one ready for another review pass or are you still working on it?

It's almost there, I'll try to finish this one up today/tomorrow.

[G-Ambatte]

5. It might make sense to reuse (and maybe update) the "notificationPopup.jsx" component we already have instead of making a separate component element.

I've just pushed some changes now that creates a new Dialog component, which the existing notificationPopup.jsx has been modified to use. However, the new Dialog component uses <dialog> instead of <div>, which allows it to include an option to effectively block the entire page until it is acknowledged. As this is not how the existing NotificationPopup has worked, this option is not enabled for those messages in the current build, but this should allow us to use the blocking version for the initial LockNotification on the EditPage, and the non-blocking version for an "on auto-save" reminder message.

---

Currently, ONLY the blue NotificationPopup uses the new Dialog, not the orange RenderWarning popup.

[G-Ambatte]

State of the PR:

Dialog components:

• [x] LockNotification
• [x] NotificationPopup
• [x] RenderWarning

To do:

• [x] Shift RenderWarning to use Dialog
• [x] Shift dialog[open] styling to new dialog.less file
• [x] Tweak NotificationPopup styling to better match pre-PR layout

These are pretty minimal changes and I don't anticipate them taking much longer.

[G-Ambatte]

I believe that's all of the outstanding items now!

[G-Ambatte]

I manually added a lock object to a brew so that the operation can be seen on the deployment; of course I realized after the fact that this does not help a great deal unless anyone who wants to test it is able to log in on an author account.

https://homebrewery-pr-3382.herokuapp.com/edit/69po5z2nejuq

[5e-Cleric]

Looks good to me.

[G-Ambatte]

FYI the Lock Notification has both onClick and onCancel methods, so it can be cleared by pressing the ESC key. This calls the same function as clicking the "CONTINUE TO EDITOR" button.

[G-Ambatte]

Final state of the PR:

When a modal (blocking notification) is shown, the rest of the page dims to indicate that it cannot be interacted with.

Regular pop ups (like Render Warning or Notification Popup) do not cause this dimming.

Currently, clicking "REQUEST LOCK REMOVAL" pops a "NYI" alert. Implementing this functionality is work for a future PR. Clicking "CONTINUE TO EDITOR" (or pressing ESC) removes the modal pop up and allows normal access to the editor. However, this pop up will return every time the page is refreshed. Possible work for a future PR: cause the Lock notification to reappear as a non-blocking notification on a relatively long timer (30 minutes, 60 minutes)

The content of the notification is fixed, with the exception of the LOCK REASON message pulled from the lock object on the brew itself (in the image, "This is the private message") which is only intended to only be visible to authors.

[5e-Cleric]

Possible work for a future PR: cause the Lock notification to reappear as a non-blocking notification on a relatively long timer (30 minutes, 60 minutes)

I disagree with this, if we lock a brew, the user first priority should be removing such content. This encourages that behaviour. Making it wait longer a longer time gives 0 advantages.