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

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

Information

Review Board
master

Reviewers

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 ID Author
Fixed publish being disabled when error occurs while saving review request field.
380ddd8571c53a98797f281e55fbbd855f6b79d0 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. Show all issues

    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. Show all issues

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

  4. Show all issues

    This should also use single quotes.

    Lines must also be less than 80 characters in length.

  5. Show all issues

    Single quotes here as well.

  6. 
      
amalik2
amalik2
amalik2
brennie
  1. 
      
  2. Show all issues

    Can you wrap your description at 72 characters?

  3. Show all issues

    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. Show all issues

    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.