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:

[calculuschild]

Some thoughts:

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?
2. The error should probably explain what being "locked" means. I.e. the brew is not visible via the share page, etc.
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".
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).

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

This might need some more discussion but I would like to hear what you think.

[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.

[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.

[G-Ambatte]

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.

This would be in addition to the existing blocking notification, so authors can't just clear the message and then never refresh the Edit page, and thus never be presented with the notification again. The timed notification would serve as an additional reminder, and potentially also provide an easier way to access the "REQUEST REVIEW" button once the required changes have been made.

[G-Ambatte]

I've run this through the Linter now, the only thing it picked up from these changes is an unused useState, which I have corrected in the latest commit.

[5e-Cleric]

This would be in addition to the existing blocking notification, so authors can't just clear the message and then never refresh the Edit page, and thus never be presented with the notification again. The timed notification would serve as an additional reminder, and potentially also provide an easier way to access the "REQUEST REVIEW" button once the required changes have been made.

Oh, then yeah, sure, great
Actually we might want to keep a floating button somewhere, with the "REQUEST REVIEW" option. I can imagine users complaining: i fixed the brew but i can't request review!

[calculuschild]

Thanks @G-Ambatte ! 💯 ⭐ 🎉