Address pull request comments

* Reformatted javascript
* Appeased pycodestyle
* TODO Add Expiration and Note fields to tokens?

.gitignore

@@ -2,6 +2,3 @@ config.yaml
 */*_modules/
 .coverage
 .cache
-__pycache__
-.idea/
-*.iml

client/html/user_tokens.tpl

@@ -10,7 +10,7 @@
         <div class="token-flex-row">
             <div><%= token.token %></div>
             <div>
-                <form id='token<%= index %>'>
+                <form class='token' data-token-id='<%= index %>'>
                     <input type='hidden' name='token' value='<%= token.token %>'/>
                     <input type='submit' value='Delete token'/>
                 </form>

client/js/api.js

@@ -89,11 +89,11 @@ class Api extends events.EventTarget {
     loginFromCookies() {
         const auth = cookies.getJSON('auth');
         return auth && auth.user && auth.token ?
-            this.login_with_token(auth.user, auth.token, true) :
+            this.loginWithToken(auth.user, auth.token, true) :
             Promise.resolve();
     }
 
-    login_with_token(userName, token, doRemember) {
+    loginWithToken(userName, token, doRemember) {
         this.cache = {};
         return new Promise((resolve, reject) => {
             this.userName = userName;
@@ -118,7 +118,7 @@ class Api extends events.EventTarget {
         });
     }
 
-    create_token(userName, options) {
+    createToken(userName, options) {
         return new Promise((resolve, reject) => {
             this.post('/user-token/' + userName, {})
                 .then(response => {
@@ -135,7 +135,7 @@ class Api extends events.EventTarget {
         });
     }
 
-    delete_token(userName, userToken) {
+    deleteToken(userName, userToken) {
         return new Promise((resolve, reject) => {
             this.delete('/user-token/' + userName + '/' + userToken, {})
                 .then(response => {
@@ -162,7 +162,7 @@ class Api extends events.EventTarget {
                     if (doRemember) {
                         options.expires = 365;
                     }
-                    this.create_token(this.userName, options);
+                    this.createToken(this.userName, options);
                     this.user = response;
                     resolve();
                     this.dispatchEvent(new CustomEvent('login'));
@@ -175,7 +175,7 @@ class Api extends events.EventTarget {
 
     logout() {
         let self = this;
-        this.delete_token(this.userName, this.userToken)
+        this.deleteToken(this.userName, this.userToken)
             .then(response => {
                 self._logout();
             }, error => {

client/js/controllers/user_controller.js

@@ -87,12 +87,12 @@ class UserController {
             this._view.addEventListener('create-token', e => this._evtCreateToken(e));
             this._view.addEventListener('delete-token', e => this._evtDeleteToken(e));
 
-            for (let i = 0; i < this._successMessages.length; i++) {
+            for (let message of this._successMessages) {
-                this.showSuccess(this._successMessages[i]);
+                this.showSuccess(message);
             }
 
-            for (let i = 0; i < this._errorMessages.length; i++) {
+            for (let message of this._errorMessages) {
-                this.showError(this._errorMessages[i]);
+                this.showError(message);
             }
 
         }, error => {

client/js/models/user_token.js

@@ -11,20 +11,20 @@ class UserToken extends events.EventTarget {
         this._updateFromResponse({});
     }
 
-    get token()               { return this._token; }
+    get token()         { return this._token; }
-    get enabled()               { return this._enabled; }
+    get enabled()       { return this._enabled; }
-    get version()               { return this._version; }
+    get version()       { return this._version; }
-    get creation_time()              { return this._creation_time; }
+    get creationTime()  { return this._creationTime; }
 
     static fromResponse(response) {
         if (typeof response.results !== 'undefined') {
-            let token_list = [];
+            let tokenList = [];
-            for (let i = 0; i < response.results.length; i++) {
+            for (let responseToken of response.results) {
                 const token = new UserToken();
-                token._updateFromResponse(response.results[i]);
+                token._updateFromResponse(responseToken);
-                token_list.push(token)
+                tokenList.push(token)
             }
-            return token_list;
+            return tokenList;
         } else {
             const ret = new UserToken();
             ret._updateFromResponse(response);
@@ -62,10 +62,10 @@ class UserToken extends events.EventTarget {
 
     _updateFromResponse(response) {
         const map = {
-            _token:             response.token,
+            _token:        response.token,
-            _enabled:           response.enabled,
+            _enabled:      response.enabled,
-            _version:           response.version,
+            _version:      response.version,
-            _creation_time:     response.creationTime,
+            _creationTime: response.creationTime,
         };
 
         Object.assign(this, map);

client/js/views/user_tokens_view.js

@@ -22,11 +22,12 @@ class UserTokenView extends events.EventTarget {
     }
 
     _decorateTokenForms() {
+        this._tokenFormNodes = [];
         for (let i = 0; i < this._tokens.length; i++) {
-            let formNode = this._hostNode.querySelector('#token' + i);
+            let formNode = this._hostNode.querySelector('.token[data-token-id=\"' + i + '\"]');
             views.decorateValidator(formNode);
             formNode.addEventListener('submit', e => this._evtDelete(e));
-            this._tokenFormNodes.push(formNode)
+            this._tokenFormNodes.push(formNode);
         }
     }
 
@@ -44,23 +45,21 @@ class UserTokenView extends events.EventTarget {
 
     enableForm() {
         views.enableForm(this._formNode);
-        for (let i = 0; i < this._tokenFormNodes.length; i++) {
+        for (let formNode of this._tokenFormNodes) {
-            let formNode = this._tokenFormNodes[i];
             views.enableForm(formNode);
         }
     }
 
     disableForm() {
         views.disableForm(this._formNode);
-        for (let i = 0; i < this._tokenFormNodes.length; i++) {
+        for (let formNode of this._tokenFormNodes) {
-            let formNode = this._tokenFormNodes[i];
             views.disableForm(formNode);
         }
     }
 
     _evtDelete(e) {
         e.preventDefault();
-        const userToken = this._tokens[parseInt(e.target.id.replace('token', ''))];
+        const userToken = this._tokens[parseInt(e.target.getAttribute('data-token-id'))];
         this.dispatchEvent(new CustomEvent('delete', {
             detail: {
                 user: this._user,

server/szurubooru/api/user_token_api.py

@@ -13,7 +13,8 @@ def _serialize(
 
 
 @rest.routes.get('/user-tokens/(?P<user_name>[^/]+)/?')
-def get_user_tokens(ctx: rest.Context, params: Dict[str, str] = {}) -> rest.Response:
+def get_user_tokens(
+        ctx: rest.Context, params: Dict[str, str] = {}) -> rest.Response:
     user = users.get_user_by_name(params['user_name'])
     infix = 'self' if ctx.user.user_id == user.user_id else 'any'
     auth.verify_privilege(ctx.user, 'user_tokens:list:%s' % infix)
@@ -24,7 +25,8 @@ def get_user_tokens(ctx: rest.Context, params: Dict[str, str] = {}) -> rest.Resp
 
 
 @rest.routes.post('/user-token/(?P<user_name>[^/]+)/?')
-def create_user_token(ctx: rest.Context, params: Dict[str, str] = {}) -> rest.Response:
+def create_user_token(
+        ctx: rest.Context, params: Dict[str, str] = {}) -> rest.Response:
     user = users.get_user_by_name(params['user_name'])
     infix = 'self' if ctx.user.user_id == user.user_id else 'any'
     auth.verify_privilege(ctx.user, 'user_tokens:create:%s' % infix)
@@ -33,27 +35,30 @@ def create_user_token(ctx: rest.Context, params: Dict[str, str] = {}) -> rest.Re
 
 
 @rest.routes.put('/user-token/(?P<user_name>[^/]+)/(?P<user_token>[^/]+)/?')
-def update_user_token(ctx: rest.Context, params: Dict[str, str] = {}) -> rest.Response:
+def update_user_token(
+        ctx: rest.Context, params: Dict[str, str] = {}) -> rest.Response:
     user = users.get_user_by_name(params['user_name'])
     infix = 'self' if ctx.user.user_id == user.user_id else 'any'
     auth.verify_privilege(ctx.user, 'user_tokens:edit:%s' % infix)
-    user_token = user_tokens.get_user_token_by_user_and_token(user, params['user_token'])
+    user_token = user_tokens.get_by_user_and_token(user, params['user_token'])
     versions.verify_version(user_token, ctx)
     versions.bump_version(user_token)
     if ctx.has_param('enabled'):
         auth.verify_privilege(ctx.user, 'user_tokens:edit:%s' % infix)
-        user_tokens.update_user_token_enabled(user_token, ctx.get_param_as_bool('enabled'))
+        user_tokens.update_user_token_enabled(user_token,
+                                              ctx.get_param_as_bool('enabled'))
     user_tokens.update_user_token_edit_time(user_token)
     ctx.session.commit()
     return _serialize(ctx, user_token)
 
 
 @rest.routes.delete('/user-token/(?P<user_name>[^/]+)/(?P<user_token>[^/]+)/?')
-def delete_user_token(ctx: rest.Context, params: Dict[str, str]) -> rest.Response:
+def delete_user_token(
+        ctx: rest.Context, params: Dict[str, str]) -> rest.Response:
     user = users.get_user_by_name(params['user_name'])
     infix = 'self' if ctx.user.user_id == user.user_id else 'any'
     auth.verify_privilege(ctx.user, 'user_tokens:delete:%s' % infix)
-    user_token = user_tokens.get_user_token_by_user_and_token(user, params['user_token'])
+    user_token = user_tokens.get_by_user_and_token(user, params['user_token'])
     if user_token is not None:
         ctx.session.delete(user_token)
         ctx.session.commit()

server/szurubooru/func/user_tokens.py

@@ -1,10 +1,13 @@
 from datetime import datetime
 from typing import Any, Optional, List, Dict, Callable
-
+from szurubooru import db, model, rest, errors
-from szurubooru import db, model, rest
 from szurubooru.func import auth, serialization, users
 
 
+class InvalidEnabledFieldError(errors.ValidationError):
+    pass
+
+
 class UserTokenSerializer(serialization.BaseSerializer):
     def __init__(
             self,
@@ -51,9 +54,11 @@ def serialize_user_token(
     return UserTokenSerializer(user_token, auth_user).serialize(options)
 
 
-def get_user_token_by_user_and_token(user: model.User, token: str) -> model.UserToken:
+def get_by_user_and_token(
+        user: model.User, token: str) -> model.UserToken:
     return (db.session.query(model.UserToken)
-            .filter(model.UserToken.user_id == user.user_id, model.UserToken.token == token)
+            .filter(model.UserToken.user_id == user.user_id,
+                    model.UserToken.token == token)
             .one_or_none())
 
 
@@ -76,9 +81,12 @@ def create_user_token(user: model.User) -> model.UserToken:
     return user_token
 
 
-def update_user_token_enabled(user_token: model.UserToken, enabled: bool) -> None:
+def update_user_token_enabled(
+        user_token: model.UserToken, enabled: bool) -> None:
     assert user_token
-    user_token.enabled = enabled if enabled is not None else True
+    if enabled is None:
+        raise InvalidEnabledFieldError('Enabled cannot be empty.')
+    user_token.enabled = enabled
 
 
 def update_user_token_edit_time(user_token: model.UserToken) -> None:

server/szurubooru/func/users.py

@@ -172,8 +172,7 @@ def get_user_count() -> int:
 
 
 def try_get_user_by_name(name: str) -> Optional[model.User]:
-    return (
+    return (db.session
-        db.session
             .query(model.User)
             .filter(sa.func.lower(model.User.name) == sa.func.lower(name))
             .one_or_none())
@@ -189,11 +188,11 @@ def get_user_by_name(name: str) -> model.User:
 def try_get_user_by_name_or_email(name_or_email: str) -> Optional[model.User]:
     return (
         db.session
-            .query(model.User)
+        .query(model.User)
-            .filter(
+        .filter(
             (sa.func.lower(model.User.name) == sa.func.lower(name_or_email)) |
             (sa.func.lower(model.User.email) == sa.func.lower(name_or_email)))
-            .one_or_none())
+        .one_or_none())
 
 
 def get_user_by_name_or_email(name_or_email: str) -> model.User:

server/szurubooru/middleware/authenticator.py

@@ -16,7 +16,7 @@ def _authenticate(username: str, password: str) -> model.User:
 def _authenticate_token(username: str, token: str) -> model.User:
     """Try to authenticate user. Throw AuthError for invalid users."""
     user = users.get_user_by_name(username)
-    user_token = user_tokens.get_user_token_by_user_and_token(user, token)
+    user_token = user_tokens.get_by_user_and_token(user, token)
     if not auth.is_valid_token(user_token):
         raise errors.AuthError('Invalid token.')
     return user

server/szurubooru/migrations/versions/a39c7f98a7fa_add_user_token_table.py

@@ -17,17 +17,18 @@ depends_on = None
 
 def upgrade():
     op.create_table('user_token',
-    sa.Column('id', sa.Integer(), nullable=False),
+                    sa.Column('id', sa.Integer(), nullable=False),
-    sa.Column('user_id', sa.Integer(), nullable=False),
+                    sa.Column('user_id', sa.Integer(), nullable=False),
-    sa.Column('token', sa.Unicode(length=36), nullable=False),
+                    sa.Column('token', sa.Unicode(length=36), nullable=False),
-    sa.Column('enabled', sa.Boolean(), nullable=False),
+                    sa.Column('enabled', sa.Boolean(), nullable=False),
-    sa.Column('creation_time', sa.DateTime(), nullable=False),
+                    sa.Column('creation_time', sa.DateTime(), nullable=False),
-    sa.Column('last_edit_time', sa.DateTime(), nullable=True),
+                    sa.Column('last_edit_time', sa.DateTime(), nullable=True),
-    sa.Column('version', sa.Integer(), nullable=False),
+                    sa.Column('version', sa.Integer(), nullable=False),
-    sa.ForeignKeyConstraint(['user_id'], ['user.id'], ondelete='CASCADE'),
+                    sa.ForeignKeyConstraint(['user_id'], ['user.id'],
-    sa.PrimaryKeyConstraint('id')
+                                            ondelete='CASCADE'),
-    )
+                    sa.PrimaryKeyConstraint('id'))
-    op.create_index(op.f('ix_user_token_user_id'), 'user_token', ['user_id'], unique=False)
+    op.create_index(op.f('ix_user_token_user_id'), 'user_token',
+                    ['user_id'], unique=False)
 
 
 def downgrade():

server/szurubooru/model/__init__.py

@@ -1,7 +1,5 @@
 from szurubooru.model.base import Base
-from szurubooru.model.user import (
+from szurubooru.model.user import (User, UserToken)
-    User,
-    UserToken)
 from szurubooru.model.tag_category import TagCategory
 from szurubooru.model.tag import Tag, TagName, TagSuggestion, TagImplication
 from szurubooru.model.post import (

server/szurubooru/tests/api/test_user_token_creating.py

@@ -11,7 +11,8 @@ def inject_config(config_injector):
     config_injector({'privileges': {'user_tokens:create:self': 'regular'}})
 
 
-def test_creating_user_token(user_token_factory, context_factory, fake_datetime):
+def test_creating_user_token(
+        user_token_factory, context_factory, fake_datetime):
     user_token = user_token_factory()
     with patch('szurubooru.func.user_tokens.create_user_token'), \
             patch('szurubooru.func.user_tokens.serialize_user_token'), \
@@ -21,9 +22,7 @@ def test_creating_user_token(user_token_factory, context_factory, fake_datetime)
         user_tokens.serialize_user_token.return_value = 'serialized user token'
         user_tokens.create_user_token.return_value = user_token
         result = api.user_token_api.create_user_token(
-            context_factory(
+            context_factory(user=user_token.user),
-                user=user_token.user),
             {'user_name': user_token.user.name})
         assert result == 'serialized user token'
-        user_tokens.create_user_token.assert_called_once_with(
+        user_tokens.create_user_token.assert_called_once_with(user_token.user)
-            user_token.user)

server/szurubooru/tests/api/test_user_token_deleting.py

@@ -11,19 +11,20 @@ def inject_config(config_injector):
     config_injector({'privileges': {'user_tokens:delete:self': 'regular'}})
 
 
-def test_deleting_user_token(user_token_factory, context_factory, fake_datetime):
+def test_deleting_user_token(
+        user_token_factory, context_factory, fake_datetime):
     user_token = user_token_factory()
     db.session.add(user_token)
     db.session.commit()
-    with patch('szurubooru.func.user_tokens.get_user_token_by_user_and_token'), \
+    with patch('szurubooru.func.user_tokens.get_by_user_and_token'), \
             patch('szurubooru.func.users.get_user_by_name'), \
             fake_datetime('1969-02-12'):
         users.get_user_by_name.return_value = user_token.user
-        user_tokens.get_user_token_by_user_and_token.return_value = user_token
+        user_tokens.get_by_user_and_token.return_value = user_token
         result = api.user_token_api.delete_user_token(
-            context_factory(
+            context_factory(user=user_token.user),
-                user=user_token.user),
+            {'user_name': user_token.user.name,
-            {'user_name': user_token.user.name, 'user_token': user_token.token})
+             'user_token': user_token.token})
         assert result == {}
-        user_tokens.get_user_token_by_user_and_token.assert_called_once_with(
+        user_tokens.get_by_user_and_token.assert_called_once_with(
             user_token.user, user_token.token)

server/szurubooru/tests/api/test_user_token_retrieving.py

@@ -11,7 +11,8 @@ def inject_config(config_injector):
     config_injector({'privileges': {'user_tokens:list:self': 'regular'}})
 
 
-def test_retrieving_user_tokens(user_token_factory, context_factory, fake_datetime):
+def test_retrieving_user_tokens(
+        user_token_factory, context_factory, fake_datetime):
     user_token1 = user_token_factory()
     user_token2 = user_token_factory(user=user_token1.user)
     user_token3 = user_token_factory(user=user_token1.user)
@@ -21,11 +22,10 @@ def test_retrieving_user_tokens(user_token_factory, context_factory, fake_dateti
             fake_datetime('1969-02-12'):
         users.get_user_by_name.return_value = user_token1.user
         user_tokens.serialize_user_token.return_value = 'serialized user token'
-        user_tokens.get_user_tokens.return_value = [user_token1, user_token2, user_token3]
+        user_tokens.get_user_tokens.return_value = [user_token1, user_token2,
+                                                    user_token3]
         result = api.user_token_api.get_user_tokens(
-            context_factory(
+            context_factory(user=user_token1.user),
-                user=user_token1.user),
             {'user_name': user_token1.user.name})
-        assert result == {'results': ['serialized user token', 'serialized user token', 'serialized user token']}
+        assert result == {'results': ['serialized user token'] * 3}
-        user_tokens.get_user_tokens.assert_called_once_with(
+        user_tokens.get_user_tokens.assert_called_once_with(user_token1.user)
-            user_token1.user)

server/szurubooru/tests/api/test_user_token_updating.py

@@ -15,7 +15,7 @@ def test_edit_user_token(user_token_factory, context_factory, fake_datetime):
     user_token = user_token_factory()
     db.session.add(user_token)
     db.session.commit()
-    with patch('szurubooru.func.user_tokens.get_user_token_by_user_and_token'), \
+    with patch('szurubooru.func.user_tokens.get_by_user_and_token'), \
             patch('szurubooru.func.user_tokens.update_user_token_enabled'), \
             patch('szurubooru.func.user_tokens.update_user_token_edit_time'), \
             patch('szurubooru.func.user_tokens.serialize_user_token'), \
@@ -23,17 +23,16 @@ def test_edit_user_token(user_token_factory, context_factory, fake_datetime):
             fake_datetime('1969-02-12'):
         users.get_user_by_name.return_value = user_token.user
         user_tokens.serialize_user_token.return_value = 'serialized user token'
-        user_tokens.get_user_token_by_user_and_token.return_value = user_token
+        user_tokens.get_by_user_and_token.return_value = user_token
         result = api.user_token_api.update_user_token(
-            context_factory(
+            context_factory(params={'version': user_token.version,
-                params={
+                                    'enabled': False,
-                    'version': user_token.version,
+                                    },
-                    'enabled': False,
+                            user=user_token.user),
-                },
+            {'user_name': user_token.user.name,
-                user=user_token.user),
+             'user_token': user_token.token})
-            {'user_name': user_token.user.name, 'user_token': user_token.token})
         assert result == 'serialized user token'
-        user_tokens.get_user_token_by_user_and_token.assert_called_once_with(
+        user_tokens.get_by_user_and_token.assert_called_once_with(
             user_token.user, user_token.token)
         user_tokens.update_user_token_enabled.assert_called_once_with(
             user_token, False)

server/szurubooru/tests/func/test_user_tokens.py

@@ -27,12 +27,13 @@ def test_serialize_user_token_none():
     assert result is None
 
 
-def test_get_user_token_by_user_and_token(user_token_factory):
+def test_get_by_user_and_token(user_token_factory):
     user_token = user_token_factory()
     db.session.add(user_token)
     db.session.flush()
     db.session.commit()
-    result = user_tokens.get_user_token_by_user_and_token(user_token.user, user_token.token)
+    result = user_tokens.get_by_user_and_token(user_token.user,
+                                               user_token.token)
     assert result == user_token
 
 

server/szurubooru/tests/middleware/test_authenticator.py

@@ -32,9 +32,9 @@ def test_process_request_token_auth_valid(context_factory, user_token_factory):
     })
     with patch('szurubooru.func.auth.is_valid_token'), \
             patch('szurubooru.func.users.get_user_by_name'), \
-            patch('szurubooru.func.user_tokens.get_user_token_by_user_and_token'):
+            patch('szurubooru.func.user_tokens.get_by_user_and_token'):
         users.get_user_by_name.return_value = user_token.user
-        user_tokens.get_user_token_by_user_and_token.return_value = user_token
+        user_tokens.get_by_user_and_token.return_value = user_token
         auth.is_valid_password.return_value = True
         authenticator.process_request(ctx)
         assert ctx.user == user_token.user