[checklist] Add checklist items from a template

Review Request #7960 — Created Feb. 13, 2016 and discarded

Information

rb-extension-pack
master

Reviewers

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_conleymike_conley

Space before {

mike_conleymike_conley

Missing semi-colon

mike_conleymike_conley

Should pass the radix to parseInt as the second argument: firstId = parseint(firstId, 10); See: http://jslinterrors.com/missing-radix-parameter/

mike_conleymike_conley

This can just be: return firstId - secondId;

mike_conleymike_conley

Probably should lowercase the "T" in tepmlate to be consistent with line 280.

mike_conleymike_conley

I wonder if it makes sense to have this be returned from the server sorted instead of making the client …

mike_conleymike_conley

Also worth filing a bug to get a better error display. alert's are usually pretty rough.

mike_conleymike_conley

Can we keep the formatting as before? With: ```JavaScript this.collection.create({ description: description, }, _.bind(function() { this._scrollToBottomOfItemList(); }, this));

mike_conleymike_conley

Please pass the radix (see my other note about this)

mike_conleymike_conley

Please define this as: parent = { description: template.get('title') };

mike_conleymike_conley

Same as above - let's try to avoid {key: var} where we can.

mike_conleymike_conley

Space before {

mike_conleymike_conley
reviewbot
  1. 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
    
    
  2. 
      
imadueme
mike_conley
  1. 
      
  2. Show all issues

    Space before {

  3. Show all issues

    Space before {

  4. Show all issues

    Missing semi-colon

  5. Show all issues

    Should pass the radix to parseInt as the second argument:

    firstId = parseint(firstId, 10);
    

    See: http://jslinterrors.com/missing-radix-parameter/

  6. Show all issues

    This can just be:

    return firstId - secondId;
    
  7. Show all issues

    Probably should lowercase the "T" in tepmlate to be consistent with line 280.

  8. Template templates! :D

  9. Show all issues

    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.

    1. Is there a way for python to sort a dictionary on the backend?

  10. Show all issues

    Also worth filing a bug to get a better error display. alert's are usually pretty rough.

    1. Maybe I should just do this bug :) https://hellosplat.com/s/beanbag/tickets/3578/

  11. Show all issues

    Can we keep the formatting as before? With:

    ```JavaScript

    this.collection.create({
    description: description,
    }, _.bind(function() {
    this._scrollToBottomOfItemList();
    }, this));

    1. Changed, but moved the last ); to a new line so that future me doesn't get confused which function is being bind. This ok?

  12. Show all issues

    Please pass the radix (see my other note about this)

  13. Show all issues

    Please define this as:

    parent = {
        description: template.get('title')
    };
    
  14. Show all issues

    Same as above - let's try to avoid {key: var} where we can.

  15. Show all issues

    Space before {

  16. 
      
imadueme
reviewbot
  1. 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
    
    
  2. 
      
imadueme
reviewbot
  1. 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
    
    
  2. 
      
david
Review request changed
Status:
Discarded