Fixed disabled buttons if an error happens updating a review field.

Review Request #10689 — Created Sept. 6, 2019 and updated

amalik2
Review Board
master
4777
reviewboard, students

The publish button ends up being disabled while the changes made to the
model are being saved, so that the user does not repeatedly click the
publish button. If the request fails due to an error with one of the
fields that were changed, then the publish button does not ever end up
being re-enabled. Now, the model emits an event when an error happens
in the above case, and the view listens for that event being published.
When the view is notified of that event, it re-enables the buttons.

I tested this locally by following the instructions listed on the bug
report page (bug #4777).

I also ran the JavaScript unit tests.
Summary Author
Fixed publish being disabled when error occurs while saving review request field.
amalik2
Description From Last Updated

Same comment here as on /r/10688 regarding the format we like for commit descriptions. Also note that you should keep ...

chipx86chipx86

Can you wrap your description at 72 characters?

brenniebrennie

We can't currently land review requests that have more than one commit (I never got around to writing that before ...

brenniebrennie

One small nit and then I'm happy. Markdown is super great in a review request, but becomes the content for ...

chipx86chipx86

We prefer single quotes over double quotes when possible (exceptions being when the text itself contains a single quote).

chipx86chipx86

This should also use single quotes. Lines must also be less than 80 characters in length.

chipx86chipx86

Single quotes here as well.

chipx86chipx86
chipx86
  1. 
      
  2. Same comment here as on /r/10688 regarding the format we like for commit descriptions.

    Also note that you should keep lines at around the 72 character point, so make sure your editor is set to wrap there.

  3. We prefer single quotes over double quotes when possible (exceptions being when the text itself contains a single quote).

  4. This should also use single quotes.

    Lines must also be less than 80 characters in length.

  5. Single quotes here as well.

  6. 
      
amalik2
amalik2
amalik2
brennie
  1. 
      
  2. Can you wrap your description at 72 characters?

  3. We can't currently land review requests that have more than one commit (I never got around to writing that before I left 😅) so can you squash your changes together?

    Even if we could, we would rather avoid fixup commits since they don't really add anything to the history.

    If you're unsure about how to squash them I'm happy to show you.

  4. 
      
amalik2
amalik2
amalik2
chipx86
  1. 
      
  2. One small nit and then I'm happy.

    Markdown is super great in a review request, but becomes the content for the commit message, and Markdown is not a standardized thing there. So the link in Testing Done is going to look strange.

    Since the bug report is already listed, and will be included in the commit message, just report the link. You can instead say "... listed on bug #4777."

    1. Oh, and the sentences should end with a period.

  3. 
      
amalik2
Review request changed

Testing Done:

   

I tested this locally by following the instructions listed on the bug

~   report page

  ~ report page (bug #4777).

   
~  

I also ran the JavaScript unit tests

  ~

I also ran the JavaScript unit tests.

Loading...