From 243ab15b8595c45c1af709b1e9b848848dc16901 Mon Sep 17 00:00:00 2001 From: rr- Date: Sun, 28 Aug 2016 19:25:15 +0200 Subject: [PATCH] server/tags: add order to tag names The better implementation of a224297. Fixes ability to reorder tag aliases, especially - the ability to change the tag's primary name after it was created. Until now, both of these scenarios needed sad workarounds on the user part. --- server/.pylintrc | 2 +- server/szurubooru/db/tag.py | 8 ++-- server/szurubooru/func/tags.py | 18 +++++++-- .../9837fc981ec7_add_order_to_tag_names.py | 39 +++++++++++++++++++ server/szurubooru/tests/conftest.py | 5 ++- server/szurubooru/tests/db/test_tag.py | 4 +- server/szurubooru/tests/func/test_tags.py | 12 ++++++ 7 files changed, 77 insertions(+), 11 deletions(-) create mode 100644 server/szurubooru/migrations/versions/9837fc981ec7_add_order_to_tag_names.py diff --git a/server/.pylintrc b/server/.pylintrc index 70f46dda..2ff7cc84 100644 --- a/server/.pylintrc +++ b/server/.pylintrc @@ -2,7 +2,7 @@ function-rgx=^_?[a-z_][a-z0-9_]{2,}$|^test_ method-rgx=^[a-z_][a-z0-9_]{2,}$|^test_ const-rgx=^[A-Z_]+$|^_[a-zA-Z_]*$ -good-names=ex,_,logger +good-names=ex,_,logger,i [variables] dummy-variables-rgx=_|dummy diff --git a/server/szurubooru/db/tag.py b/server/szurubooru/db/tag.py index 94b1c2c5..e0861312 100644 --- a/server/szurubooru/db/tag.py +++ b/server/szurubooru/db/tag.py @@ -59,9 +59,11 @@ class TagName(Base): tag_id = Column( 'tag_id', Integer, ForeignKey('tag.id'), nullable=False, index=True) name = Column('name', Unicode(64), nullable=False, unique=True) + order = Column('ord', Integer, nullable=False, index=True) - def __init__(self, name): + def __init__(self, name, order): self.name = name + self.order = order class Tag(Base): @@ -84,7 +86,7 @@ class Tag(Base): 'TagName', cascade='all,delete-orphan', lazy='joined', - order_by='TagName.tag_name_id') + order_by='TagName.order') suggestions = relationship( 'Tag', secondary='tag_suggestion', @@ -106,7 +108,7 @@ class Tag(Base): first_name = column_property( select([TagName.name]) .where(TagName.tag_id == tag_id) - .order_by(TagName.tag_name_id) + .order_by(TagName.order) .limit(1) .as_scalar(), deferred=True) diff --git a/server/szurubooru/func/tags.py b/server/szurubooru/func/tags.py index 73257148..7a4887aa 100644 --- a/server/szurubooru/func/tags.py +++ b/server/szurubooru/func/tags.py @@ -36,6 +36,8 @@ class InvalidTagDescriptionError(errors.ValidationError): def _verify_name_validity(name): + if util.value_exceeds_column_size(name, db.TagName.name): + raise InvalidTagNameError('Name is too long.') name_regex = config.config['tag_name_regex'] if not re.match(name_regex, name): raise InvalidTagNameError('Name must satisfy regex %r.' % name_regex) @@ -250,16 +252,17 @@ def update_tag_category_name(tag, category_name): def update_tag_names(tag, names): + # sanitize assert tag names = util.icase_unique([name for name in names if name]) if not len(names): raise InvalidTagNameError('At least one name must be specified.') for name in names: _verify_name_validity(name) + + # check for existing tags expr = sqlalchemy.sql.false() for name in names: - if util.value_exceeds_column_size(name, db.TagName.name): - raise InvalidTagNameError('Name is too long.') expr = expr | (sqlalchemy.func.lower(db.TagName.name) == name.lower()) if tag.tag_id: expr = expr & (db.TagName.tag_id != tag.tag_id) @@ -267,12 +270,21 @@ def update_tag_names(tag, names): if len(existing_tags): raise TagAlreadyExistsError( 'One of names is already used by another tag.') + + # remove unwanted items for tag_name in tag.names[:]: if not _check_name_intersection([tag_name.name], names, True): tag.names.remove(tag_name) + # add wanted items for name in names: if not _check_name_intersection(_get_names(tag), [name], True): - tag.names.append(db.TagName(name)) + tag.names.append(db.TagName(name, None)) + + # set alias order to match the request + for i, name in enumerate(names): + for tag_name in tag.names: + if tag_name.name.lower() == name.lower(): + tag_name.order = i # TODO: what to do with relations that do not yet exist? diff --git a/server/szurubooru/migrations/versions/9837fc981ec7_add_order_to_tag_names.py b/server/szurubooru/migrations/versions/9837fc981ec7_add_order_to_tag_names.py new file mode 100644 index 00000000..10969fe6 --- /dev/null +++ b/server/szurubooru/migrations/versions/9837fc981ec7_add_order_to_tag_names.py @@ -0,0 +1,39 @@ +''' +Add order to tag names + +Revision ID: 9837fc981ec7 +Created at: 2016-08-28 19:03:59.831527 +''' + +import sqlalchemy as sa +from alembic import op + + +revision = '9837fc981ec7' +down_revision = '4a020f1d271a' +branch_labels = None +depends_on = None + + +Base = sa.ext.declarative.declarative_base() + + +class TagName(Base): + __tablename__ = 'tag_name' + __table_args__ = {'extend_existing': True} + + tag_name_id = sa.Column('tag_name_id', sa.Integer, primary_key=True) + ord = sa.Column('ord', sa.Integer, nullable=False, index=True) + + +def upgrade(): + op.add_column('tag_name', sa.Column('ord', sa.Integer(), nullable=True)) + op.execute(TagName.__table__.update().values(ord=TagName.tag_name_id)) + op.alter_column('tag_name', 'ord', nullable=False) + op.create_index( + op.f('ix_tag_name_ord'), 'tag_name', ['ord'], unique=False) + + +def downgrade(): + op.drop_index(op.f('ix_tag_name_ord'), table_name='tag_name') + op.drop_column('tag_name', 'ord') diff --git a/server/szurubooru/tests/conftest.py b/server/szurubooru/tests/conftest.py index d1431c94..1f408877 100644 --- a/server/szurubooru/tests/conftest.py +++ b/server/szurubooru/tests/conftest.py @@ -145,8 +145,9 @@ def tag_factory(): category = db.TagCategory(get_unique_name()) db.session.add(category) tag = db.Tag() - tag.names = [ - db.TagName(name) for name in names or [get_unique_name()]] + tag.names = [] + for i, name in enumerate(names or [get_unique_name()]): + tag.names.append(db.TagName(name, i)) tag.category = category tag.creation_time = datetime(1996, 1, 1) return tag diff --git a/server/szurubooru/tests/db/test_tag.py b/server/szurubooru/tests/db/test_tag.py index 1a98ab2e..02134d69 100644 --- a/server/szurubooru/tests/db/test_tag.py +++ b/server/szurubooru/tests/db/test_tag.py @@ -8,7 +8,7 @@ def test_saving_tag(tag_factory): imp1 = tag_factory(names=['imp1']) imp2 = tag_factory(names=['imp2']) tag = db.Tag() - tag.names = [db.TagName('alias1'), db.TagName('alias2')] + tag.names = [db.TagName('alias1', 0), db.TagName('alias2', 1)] tag.suggestions = [] tag.implications = [] tag.category = db.TagCategory('category') @@ -49,7 +49,7 @@ def test_cascade_deletions(tag_factory): imp1 = tag_factory(names=['imp1']) imp2 = tag_factory(names=['imp2']) tag = db.Tag() - tag.names = [db.TagName('alias1'), db.TagName('alias2')] + tag.names = [db.TagName('alias1', 0), db.TagName('alias2', 1)] tag.suggestions = [] tag.implications = [] tag.category = db.TagCategory('category') diff --git a/server/szurubooru/tests/func/test_tags.py b/server/szurubooru/tests/func/test_tags.py index 58077aec..f438a9dc 100644 --- a/server/szurubooru/tests/func/test_tags.py +++ b/server/szurubooru/tests/func/test_tags.py @@ -483,6 +483,18 @@ def test_update_tag_names_reusing_own_name(config_injector, tag_factory): db.session.rollback() +def test_update_tag_names_changing_primary_name(config_injector, tag_factory): + config_injector({'tag_name_regex': '^[a-zA-Z]*$'}) + tag = tag_factory(names=['a', 'b']) + db.session.add(tag) + db.session.flush() + tags.update_tag_names(tag, ['b', 'a']) + db.session.flush() + db.session.refresh(tag) + assert [tag_name.name for tag_name in tag.names] == ['b', 'a'] + db.session.rollback() + + @pytest.mark.parametrize('attempt', ['name', 'NAME', 'alias', 'ALIAS']) def test_update_tag_suggestions_with_itself(attempt, tag_factory): tag = tag_factory(names=['name', 'ALIAS'])