BackBone.JS Models for File Provider framework

Review Request #7140 — Created March 31, 2015 and updated

VTL-Developer
Review Board
master
7124
7009, 7144
8b34ae7...
reviewboard, students

In order to make use of responses from the File Provider framework (via api calls), BackBone.JS models can be defined to abstract these responses. These models are extended from RB.BaseResource, which include base implementation such as fetch, and parse to interact with the api calls properly. As they are BackBone.JS models, functionality can be defined on top of the object as needed, such as initialization and serialization.

Test cases for each model was implemented (static/rb/js/resources/models/tests/*.js), each one passed.

Description From Last Updated

id is a reserved attribute for Django models. Assuming this id maps to the one in the database, you don't …

chipx86chipx86

Trailing commas in dictionaries or lists will break certain browsers.

chipx86chipx86

id doesn't need to be included in this list or the next one.

chipx86chipx86

Most things will actually have a type of "object". I would instead have this be: if (!options || !options.credentials) { …

chipx86chipx86

In JavaScript, you should almost never use new Object(). Instead, just pass {}.

chipx86chipx86

Hmm, looks like we're using id to mean something else here. We should pick a name other than id. Also, …

chipx86chipx86

Same comment about id.

chipx86chipx86

Same concerns about id here and below.

chipx86chipx86

Many things will have a type of "object". You should use _.isObject() instead.

chipx86chipx86

Should this default to false, rather than null?

daviddavid
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/fileProviderAccountModel.js
        reviewboard/static/rb/js/resources/models/tests/fileProviderAccountModelTests.js
        reviewboard/static/rb/js/resources/models/fileProviderNodeModel.js
        reviewboard/static/rb/js/resources/models/tests/fileProviderModelTests.js
        reviewboard/static/rb/js/resources/models/tests/fileProviderNodeModelTests.js
        reviewboard/static/rb/js/resources/models/fileProviderModel.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/fileProviderAccountModel.js
        reviewboard/static/rb/js/resources/models/tests/fileProviderAccountModelTests.js
        reviewboard/static/rb/js/resources/models/fileProviderNodeModel.js
        reviewboard/static/rb/js/resources/models/tests/fileProviderModelTests.js
        reviewboard/static/rb/js/resources/models/tests/fileProviderNodeModelTests.js
        reviewboard/static/rb/js/resources/models/fileProviderModel.js
    
    
  2. 
      
VT
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/fileProviderAccountModel.js
        reviewboard/static/rb/js/resources/models/tests/fileProviderAccountModelTests.js
        reviewboard/static/rb/js/resources/utils/serializers.js
        reviewboard/static/rb/js/resources/models/fileProviderNodeModel.js
        reviewboard/static/rb/js/resources/models/tests/fileProviderModelTests.js
        reviewboard/static/rb/js/resources/models/tests/fileProviderNodeModelTests.js
        reviewboard/static/rb/js/resources/models/fileProviderModel.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/fileProviderAccountModel.js
        reviewboard/static/rb/js/resources/models/tests/fileProviderAccountModelTests.js
        reviewboard/static/rb/js/resources/utils/serializers.js
        reviewboard/static/rb/js/resources/models/fileProviderNodeModel.js
        reviewboard/static/rb/js/resources/models/tests/fileProviderModelTests.js
        reviewboard/static/rb/js/resources/models/tests/fileProviderNodeModelTests.js
        reviewboard/static/rb/js/resources/models/fileProviderModel.js
    
    
  2. 
      
chipx86
  1. 
      
  2. id is a reserved attribute for Django models. Assuming this id maps to the one in the database, you don't need it here.

  3. Trailing commas in dictionaries or lists will break certain browsers.

  4. id doesn't need to be included in this list or the next one.

  5. Most things will actually have a type of "object". I would instead have this be:

    if (!options || !options.credentials) {
        ...
    }
    
  6. In JavaScript, you should almost never use new Object(). Instead, just pass {}.

  7. Hmm, looks like we're using id to mean something else here. We should pick a name other than id.

    Also, it doesn't look like this is matching the deserializedAttrs below?

  8. Same concerns about id here and below.

  9. Many things will have a type of "object". You should use _.isObject() instead.

  10. 
      
VT
Review request changed

Change Summary:

Updated how ids are stored for RB.FileProviderNode and RB.FileProvider using idAttribute.

Remove id property and parsing from RB.FileProviderAccount.

Removed all unused path_sep logic in tests and model.

Updated the serializer flattenJSON to use a better Object verifier.

Commit:

-794af31a51813684dc846f1123dd8ee327c6bd54
+8b34ae7d4b3629e8f6c286258d2e984abe68c086

Diff:

Revision 3 (+315)

Show changes

reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/fileProviderAccountModel.js
        reviewboard/static/rb/js/resources/models/tests/fileProviderAccountModelTests.js
        reviewboard/static/rb/js/resources/utils/serializers.js
        reviewboard/static/rb/js/resources/models/fileProviderNodeModel.js
        reviewboard/static/rb/js/resources/models/tests/fileProviderModelTests.js
        reviewboard/static/rb/js/resources/models/tests/fileProviderNodeModelTests.js
        reviewboard/static/rb/js/resources/models/fileProviderModel.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/resources/models/fileProviderAccountModel.js
        reviewboard/static/rb/js/resources/models/tests/fileProviderAccountModelTests.js
        reviewboard/static/rb/js/resources/utils/serializers.js
        reviewboard/static/rb/js/resources/models/fileProviderNodeModel.js
        reviewboard/static/rb/js/resources/models/tests/fileProviderModelTests.js
        reviewboard/static/rb/js/resources/models/tests/fileProviderNodeModelTests.js
        reviewboard/static/rb/js/resources/models/fileProviderModel.js
    
    
  2. 
      
david
  1. 
      
  2. Should this default to false, rather than null?

  3. 
      
Loading...