connor-prodege/gitlabhq — pull request #5 by AlexanderOlkhovskiy (2013-10-30 13:48:12+00:00)
Title: More merge request review states

See https://prodegellc.atlassian.net/browse/SWAG-486 ("GitLab: more merge request states")

The branch consists of 4 commits:
* 1dfefb4e17bdb64cf867f33fcb0dc4fce061fcd1 ("Fix: show edit/close buttons for all not closed MRs (not only to opened ones)"): this fixes a bug introduced in https://github.com/connor-prodege/gitlabhq/pull/3 : it was impossible to edit or close MR after accepting it without merge; fix is also valid for the new states introduced in the next commit:
* 21154576a75ac9c6f13607576f1376707253a3cc ("Add 'rejected' and 'fixed' states to merge requests; allow to switch between the review statuses (accepted, rejected, fixed) back and forth"): this is actually the implementation of JIRA task; there are very similar changes to ones in https://github.com/connor-prodege/gitlabhq/pull/3
* 10a914bc4f2f0e997bf1e59d68236b22d641a980 ("Allow to filter merge requests by state"): should be self-explaining, changes and functionality are similar to https://github.com/connor-prodege/gitlabhq/pull/2
* e61df77b1538654b4bc69e62b24ae368e493fcfd ("Show name of user who changed MR's review state in email subject and body") - I initially copied the "merged" email for "accept without merge" one, then generalized it for other review statuses in 21154576a75ac9c6f13607576f1376707253a3cc . But all these new emails are confusing without user names, so I added them in this commit.

Additional notes:
* https://github.com/connor-prodege/gitlabhq/pull/4 is deprecated by this pull request (this one doing the same plus adds more of related functionality).
* Merge itself is allowed from any new state, it would be more logical to allow it only from the accept state, but I didn't want to annoy people by introducing new restrictions right from the initial implementation (we can adjust things after some "test driving").
* Small known issue: Firefox caches the disabled button state ('disabled' attribute for the 'input' element) for the new buttons ('Accept without merge', 'Reject', 'Mark fixed') on page reload; so after e.g. clicking 'Accept without merge' page will be reloaded automatically and 'Reject' button will be disabled (it shouldn't). If page is reloaded with cache override (Ctrl+F5) or is opened in another tab, all buttons will be enabled (as they should). In Chrome it works fine. I wasn't able to quickly find how to fix this and I'm not sure that the issue will occur on production (maybe it will gone due to different caching setup on server side). Also it isn't the typical use case to click these buttons one after another in a row. So I decided to create the pull request as is and return to this later if it would be needed.