[checklist] Add checklist items from a template
Review Request #7960 — Created Feb. 13, 2016 and discarded
Background
Previous work on the checklist allowed templates to be created, this change
finishes the ability to actually import checklist items from a template.
Please see this gif for a demonstration. Note: These changes depend on the
changes in 7248. 7248 implemented single level nesting of checklist items.Changes
A button was added that allows the user to switch input modes between adding
a single item with text, or selecting one of the user's templates and
importing those items into the checklist for the current review request.
The templates can be created in the user account page.In additon, before when the user would add items and the page was refreshed
the items would all of a sudden be in different locations. This change also
improves the sorting of checklist items and initial load of the items so
that the items remain in the order they were created.Other changes:
Ignored PyCharm (.idea) folder in .gitignore.
Only manual testing was done as described below.
Switch input modes
- Load a diff page, open the checklist.
- Click the "Add items from a template" link.
- The input box on the bottom should switch to a select box and add button.
- The link should now say "Add individual items".
- Clicking on it now should bring back the text input box.
- The link and input modes toggle multiple times as specified above.
Load template options
- Create a new template in the user account page.
- Load a diff page, and open the checklist template select.
- The new template title should be present and selectable.
Add template items
- Open a checklist.
- Select a template to add and click add.
- It should add the items, with the title of the template as the parent item.
- The items of the template should be child items nested under this parent.
- Refresh the page, the items should still be there.
- Try adding a combination of indivual items and multiple templates.
- You should not be able to add the default "Select a Template" option.
Load in order
- Have a blank checklist.
- Add items individually, and through the template. Take note of the order of the items.
- Refresh the page, the items should be in the same order they once were.
Regression
- All the buttons to check, add, edit, remove, collapse, etc, should work.
- Adding just individual items should still work.
- Checking items within a template should work.
| Description | From | Last Updated |
|---|---|---|
|
Space before { |
 mike_conley
|
|
|
Space before { |
mike_conley
|
|
|
Missing semi-colon |
mike_conley
|
|
|
Should pass the radix to parseInt as the second argument: firstId = parseint(firstId, 10); See: http://jslinterrors.com/missing-radix-parameter/ |
mike_conley
|
|
|
This can just be: return firstId - secondId; |
mike_conley
|
|
|
Probably should lowercase the "T" in tepmlate to be consistent with line 280. |
mike_conley
|
|
|
I wonder if it makes sense to have this be returned from the server sorted instead of making the client … |
mike_conley
|
|
|
Also worth filing a bug to get a better error display. alert's are usually pretty rough. |
mike_conley
|
|
|
Can we keep the formatting as before? With: ```JavaScript this.collection.create({ description: description, }, _.bind(function() { this._scrollToBottomOfItemList(); }, this)); |
mike_conley
|
|
|
Please pass the radix (see my other note about this) |
mike_conley
|
|
|
Please define this as: parent = { description: template.get('title') }; |
mike_conley
|
|
|
Same as above - let's try to avoid {key: var} where we can. |
mike_conley
|
|
|
Space before { |
mike_conley
|
-
-
-
-
-
checklist/checklist/static/js/models/checklist.js (Diff revision 1) Should pass the radix to parseInt as the second argument:
firstId = parseint(firstId, 10);See: http://jslinterrors.com/missing-radix-parameter/
-
checklist/checklist/static/js/models/checklist.js (Diff revision 1) This can just be:
return firstId - secondId; -
checklist/checklist/static/js/views/checklistView.js (Diff revision 1) Probably should lowercase the "T" in tepmlate to be consistent with line 280.
-
-
checklist/checklist/static/js/views/checklistView.js (Diff revision 1) I wonder if it makes sense to have this be returned from the server sorted instead of making the client do it. No need to fix that here, but worth filing a bug about.
-
checklist/checklist/static/js/views/checklistView.js (Diff revision 1) Also worth filing a bug to get a better error display. alert's are usually pretty rough.
-
checklist/checklist/static/js/views/checklistView.js (Diff revision 1) Can we keep the formatting as before? With:
```JavaScript
this.collection.create({
description: description,
}, _.bind(function() {
this._scrollToBottomOfItemList();
}, this)); -
checklist/checklist/static/js/views/checklistView.js (Diff revision 1) Please pass the radix (see my other note about this)
-
checklist/checklist/static/js/views/checklistView.js (Diff revision 1) Please define this as:
parent = { description: template.get('title') }; -
checklist/checklist/static/js/views/checklistView.js (Diff revision 1) Same as above - let's try to avoid
{key: var}where we can. -
-
Tool: Pyflakes Processed Files: checklist/checklist/models.py checklist/checklist/resources/checklist_item.py checklist/checklist/extension.py Ignored Files: checklist/checklist/static/js/views/checklistView.js checklist/checklist/static/css/style.less checklist/checklist/static/js/models/checklist.js .gitignore Tool: PEP8 Style Checker Processed Files: checklist/checklist/models.py checklist/checklist/resources/checklist_item.py checklist/checklist/extension.py Ignored Files: checklist/checklist/static/js/views/checklistView.js checklist/checklist/static/css/style.less checklist/checklist/static/js/models/checklist.js .gitignore
-
Tool: PEP8 Style Checker Processed Files: checklist/checklist/extension.py Ignored Files: checklist/checklist/static/js/views/checklistView.js checklist/checklist/static/css/style.less checklist/checklist/static/js/models/checklist.js .gitignore Tool: Pyflakes Processed Files: checklist/checklist/extension.py Ignored Files: checklist/checklist/static/js/views/checklistView.js checklist/checklist/static/css/style.less checklist/checklist/static/js/models/checklist.js .gitignore
Tool: Pyflakes Processed Files: checklist/checklist/extension.py Ignored Files: checklist/checklist/static/js/views/checklistView.js checklist/checklist/static/css/style.less checklist/checklist/static/js/models/checklist.js .gitignore Tool: PEP8 Style Checker Processed Files: checklist/checklist/extension.py Ignored Files: checklist/checklist/static/js/views/checklistView.js checklist/checklist/static/css/style.less checklist/checklist/static/js/models/checklist.js .gitignore