500 Server Error When Commenting #65

Closed
opened 2024-03-31 21:04:27 +02:00 by Victor Chedeville · 4 comments

I get a 500 Server Error when commenting on an addon in the approval queue

Windows 11
Chromium web browser (Vivaldi)

I get a `500 Server Error` when commenting on an addon in the approval queue Windows 11 Chromium web browser (Vivaldi)

Can confirm when starting the message with a space:

Can confirm when starting the message with a space: <video src="/attachments/b7adbf73-c821-4bbe-943f-22da427402e4" title="Screen Recording 2024-04-02 at 16.31.58.mov" controls></video>
Pablo Vazquez added the
Type
Report
label 2024-04-02 16:33:57 +02:00
Owner

A bit of a side question:

I've noticed that the comment form is also displayed for users who are not maintainers of a given extension nor moderators. Is it intentional?

If we want other people to leave comments on the queue, what would be the use case?

==

500 error happens because we don't have a way to render the form submission errors:

  1. these "passing by" (not maintainers, not moderators) users mentioned above trigger an error because they don't have the type select field, so the submitted form is lacking a mandatory field
  2. a blank message (only spaces) also triggers an error

There are two steps to fix:

  1. make sure that client-side validation catches all the common cases, so that we never have to render a server-side error page: we need to block form submission is the message is blank, hide the form completely from "passers-by" or pass a hidden type="CMT" for them if we want them to comment.
  2. render a server-side form error page. it would make sense to render it in the full context of the original /approval-queue/<slug>/ where the form is submitted.

It's best to combine both for a smooth UX and for handling anything unexpected, but we could start with just the second one, especially if we don't want to allow the passer-by comments.

A bit of a side question: I've noticed that the comment form is also displayed for users who are not maintainers of a given extension nor moderators. Is it intentional? If we want other people to leave comments on the queue, what would be the use case? == 500 error happens because we don't have a way to render the form submission errors: 1. these "passing by" (not maintainers, not moderators) users mentioned above trigger an error because they don't have the `type` select field, so the submitted form is lacking a mandatory field 2. a blank message (only spaces) also triggers an error There are two steps to fix: 1. make sure that client-side validation catches all the common cases, so that we never have to render a server-side error page: we need to block form submission is the message is blank, hide the form completely from "passers-by" or pass a hidden `type="CMT"` for them if we want them to comment. 2. render a server-side form error page. it would make sense to render it in the full context of the original `/approval-queue/<slug>/` where the form is submitted. It's best to combine both for a smooth UX and for handling anything unexpected, but we could start with just the second one, especially if we don't want to allow the passer-by comments.

I've noticed that the comment form is also displayed for users who are not maintainers of a given extension nor moderators. Is it intentional?

Yes. Maybe @dfelinto can confirm, but I believe permissions go as follows:

  • Leave a Comment: Anyone. Every user is welcome to help the review process by commenting.
  • Mark as Awaiting Review: Only extension maintainers and moderators.
  • Mark as Awaiting Changes: Only extension maintainers and moderators. Used to signal that the review is on hold.
  • Mark as Approved: Only moderators.
> I've noticed that the comment form is also displayed for users who are not maintainers of a given extension nor moderators. Is it intentional? Yes. Maybe @dfelinto can confirm, but I believe permissions go as follows: * Leave a **Comment**: Anyone. Every user is welcome to help the review process by commenting. * Mark as **Awaiting Review**: Only extension maintainers and moderators. * Mark as **Awaiting Changes**: Only extension maintainers and moderators. Used to signal that the review is on hold. * Mark as **Approved**: Only moderators.

I've noticed that the comment form is also displayed for users who are not maintainers of a given extension nor moderators. Is it intentional?

Definitively intentional. Everyone is welcome to help with moderation (by commenting). This is also how we will eventually spot new moderators to join the team.


In short, what Pablo listed here is correct.

> I've noticed that the comment form is also displayed for users who are not maintainers of a given extension nor moderators. Is it intentional? Definitively intentional. Everyone is welcome to help with moderation (by commenting). This is also how we will eventually spot new moderators to join the team. ---------- In short, what Pablo listed here is correct.
Sign in to join this conversation.
No Milestone
No project
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: infrastructure/extensions-website#65
No description provided.