Add watchers #20
Add watchers #20
Conversation
TemplateTestCase provides the bare minimum needed to test templates through rendering.
A watcher or list of watchers can be assigned to pull requests. Watchers are listed as alternate owners of the request and are pinged wherever the original requester would be pinged.
Additionally protect push buttons from random users
authorized_to_manage_requests returns True for: request owner request watchers pushmaster
class="mine" is a visual indicator for request ownership in the request lists. Include watchers as well.
| requester_edit_button_text = ['Edit', 'Delay', 'Discard'] | ||
| edit_button_classes = ['edit-request'] | ||
| edit_button_text = ['Edit'] | ||
|
|
There was a problem hiding this comment.
All these buttons need to be verified to be displayed on the website as well. Some of these don't show up because of CSS. For example, the push_button_classes aren't showing up for pushmaster.
There was a problem hiding this comment.
Only made un-pickme visible in the pickme state with 8a0e3a3. The other one's are redundant with the pushmaster-only buttons.
There was a problem hiding this comment.
This might be testable with something like webdriver + phantomjs, which would be super cool but is probably out of scope for now.
| @@ -87,7 +87,7 @@ $(function() { | |||
| }); | |||
| $('.message-people').live('click', function() { | |||
| var contents = $(this).siblings('.item-count').text(); | |||
| var people = (/(?:[a-z]+,?\s?)+/.exec(contents) || [""])[0]; | |||
| var people = (/(?:[a-z]+(?:\s\((?:[a-z]+,?\s?)+\))?,?\s?)+/.exec(contents) || [""])[0]; | |||
There was a problem hiding this comment.
Uh... can we unpack this? This is crazypants.
Does js allow multi-line regex or anything else to make this easier to digest? At least can we separate the regex insanity from the other stuff that's going on (calling exec, assigning to people)?
There was a problem hiding this comment.
I've found that vanilla javascript doesnt support the /x verbose mode of regexes though there is an alternative XRegexp.
There was a problem hiding this comment.
That's fine about /x. lol javascript.
Please still unpack the other complexity from the line.
|
This looks good, modulo my complaints above. I really like the template refactoring and template unit tests! |
|
These changes look good to me. @milki++ for template tests! What's the plan to resolve the conflict & merge this in? |
|
I'll merge master into watchers and fix conflicts. |
|
Also, I need to provide a sqlscript for both sqlite3 and mysql and test an actual migration. |
Conflicts: testing/testservlet.py tests/test_template_newrequest.py
| var people_pat = new RegExp( // person, person (person, person), person (person), person | ||
| "(?:[a-z]+" + // A username, possibly followed by: | ||
| "(?:\\s\\(" + // a space and ( | ||
| "(?:[a-z]+,?\\s?)+" + // and or more comma seperated usernames |
There was a problem hiding this comment.
Probably meant to say one or more here. Also probably good to mention this in the form.
|
I've realized I've also forgotten to email watchers on request status changes. |
|
There are two outstanding issues beyond are beyond the scope of this PR: #26 Duplicate emails which needs to be investigated. #25 Prevents testing for the CommentRequest servlet. Thus, it has not been included in this PR. As mentioned above, javascript testing and template styling issues is beyond the scope of this PR as well. Other than the above issues which can be addressed later, this PR is ready for review. |
| @@ -1,3 +1,14 @@ | |||
| 2013.07.29 | |||
| AFFECTS: Users with existing installs before commit $add_watchers-merge$ | |||
There was a problem hiding this comment.
TODO: Update this to the merge commit sha after the merge.
|
Ship it! |
A watcher or list of watchers can be assigned to pull requests. Watchers are listed as alternate owners of the request and are pinged wherever the original requester would be pinged.
A watcher or list of watchers can be assigned to pull requests. Watchers are listed as alternate owners of the request and are pinged wherever the original requester would be pinged.
This PR includes a new test framework for templates. In doing so, some templates were refactored for easier template testing.
Unfortunately, the javascript changes do not have automatic tests. They have been verified through smoke testing with a local test instance and IRC.
Note: This also encompasses the feature request in PR #16