From 144e816d799dbb868ccac1e6cf702d99aa4f79e2 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Dec 09 2016 15:55:02 +0000 Subject: [PATCH 1/4] Issue 1544 - RFE - Add colored tags I only extended the base Tag object to include color, projectID, and a unique id. Then I removed the dependency of the issue from the tag for working in the ui. Otherwise tags were only visible if an issue contained that tag. This also meant you could not add, edit, or remove a tag unless you had already added one to an issue. So in my patch it updated the setting.html page to show all tags (whether or not they existed in an issue). You can add, edit and delete them. They also automatically update all the issues in that project with the color/name change. This is also reflected in both issues.html & issue.html. I also added a unique identifier to each tag(tag_id) - this allows every project to use the same name with its own unique color. There are no conflicts since the primary key is now the unique identifier(tag_id) instead of the tag name(tag). This patch enforces that tags are only created from the settings page. This patch is missing the complete alembic migration script. --- diff --git a/alembic/versions/2c81d9228a35_add_tag_color_to_tags_table.py b/alembic/versions/2c81d9228a35_add_tag_color_to_tags_table.py new file mode 100644 index 0000000..026bb45 --- /dev/null +++ b/alembic/versions/2c81d9228a35_add_tag_color_to_tags_table.py @@ -0,0 +1,51 @@ +"""add tag_color to tags table + +Revision ID: 2c81d9228a35 +Revises: 5083efccac7 +Create Date: 2016-11-21 15:22:09.793435 + +""" + +# revision identifiers, used by Alembic. +revision = '2c81d9228a35' +down_revision = '5083efccac7' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + ''' Add the column "tag_color" to the table "tags". + ''' + op.add_column( + 'tag_id', + sa.Column('tag_id', sa.String(25), primary_key=True) + ) + op.add_column( + 'tag_color', + sa.Column('tag_color', sa.String(25), default="DeepSkyBlue") + ) + op.add_column( + 'project_id', + sa.Column(sa.Integer, nullable=False) + ) + + ''' Need to update/replace every tag with the new parameters tag_id, + project_id, and tag_color. Multiple projects could be sharing the same tag + name. So we might need to create many new tags of the same name, but + different tag_id's. + + The tag ID (primary_key) is the tag name plus the project id: + + _ = myNewTag_27 + + So we need to check all the TagIssues, find its project id's, and create a + new tag (with the tag_id as the primary key). Then we need to remove the + old Tags. Easy, right? + ''' + + +def downgrade(): + ''' Remove the column "tag_color" from the table "tags". + ''' + op.drop_column('tag_id', 'tags', 'tag_color', 'project_id') diff --git a/pagure/forms.py b/pagure/forms.py index 71e6542..acc89dd 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -34,6 +34,8 @@ STRICT_REGEX = '^[a-zA-Z0-9-_]+$' TAGS_REGEX = '^[a-zA-Z0-9-_, .]+$' PROJECT_NAME_REGEX = \ '^[a-zA-z0-9_][a-zA-Z0-9-_]*$' +TAG_COLOR_LIST = ['DeepSkyBlue', 'red', 'maroon', 'SlateBlue', 'teal', 'green', + 'brown', 'orange', 'gray', 'purple', 'black'] class PagureForm(FlaskForm): @@ -225,7 +227,7 @@ class RemoteRequestPullForm(RequestPullForm): class AddIssueTagForm(PagureForm): - ''' Form to add a comment to an issue. ''' + ''' Form to add a tag to an issue. ''' tag = wtforms.TextField( 'tag', [ @@ -234,6 +236,17 @@ class AddIssueTagForm(PagureForm): wtforms.validators.Length(max=255), ] ) + tag_color = wtforms.SelectField( + 'tag_color', + [wtforms.validators.Optional()], + choices=[] + ) + + def __init__(self, *args, **kwargs): + super(AddIssueTagForm, self).__init__(*args, **kwargs) + self.tag_color.choices = [ + (tag_color, tag_color) for tag_color in TAG_COLOR_LIST + ] class StatusForm(PagureForm): @@ -286,7 +299,6 @@ class NewTokenForm(PagureForm): ] - class UpdateIssueForm(PagureForm): ''' Form to add a comment to an issue. ''' tag = wtforms.TextField( diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 38886c1..3bff234 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -331,7 +331,15 @@ def add_tag_obj(session, obj, tags, user, ticketfolder): tagobj = get_tag(session, objtag) if not tagobj: - tagobj = model.Tag(tag=objtag) + if isinstance(obj, model.Project): + project_id = obj.id + else: + project_id = obj.project.id + + tagobj = model.Tag(tag=objtag, + tag_color="DeepSkyBlue", + project_id=project_id) + session.add(tagobj) session.flush() @@ -339,6 +347,7 @@ def add_tag_obj(session, obj, tags, user, ticketfolder): dbobjtag = model.TagIssue( issue_uid=obj.uid, tag=tagobj.tag, + tag_color=tagobj.tag_color ) if isinstance(obj, model.Project): dbobjtag = model.TagProject( @@ -639,17 +648,27 @@ def remove_tags(session, project, tags, ticketfolder, user): msgs = [] removed_tags = [] - if not issues: + tag_found = False + for tag in tags: + deltags = session.query(model.Tag).filter( + model.Tag.tag == tag).filter( + model.Tag.project_id == project.id).all() + for deltag in deltags: + tag_found = True + removed_tags.append(tag) + msgs.append('Removed tag: %s' % tag) + session.delete(deltag) + + if not tag_found: raise pagure.exceptions.PagureException( - 'No issue found with the tags: %s' % ', '.join(tags)) - else: + 'Tags not found: %s' % ', '.join(tags)) + + if issues is not None: for issue in issues: for issue_tag in issue.tags: if issue_tag.tag in tags: tag = issue_tag.tag - removed_tags.append(tag) session.delete(issue_tag) - msgs.append('Removed tag: %s' % tag) pagure.lib.git.update_git( issue, repo=issue.project, repofolder=ticketfolder) @@ -667,8 +686,7 @@ def remove_tags(session, project, tags, ticketfolder, user): return msgs -def remove_tags_obj( - session, obj, tags, ticketfolder, user): +def remove_tags_obj(session, obj, tags, ticketfolder, user): ''' Removes the specified tag(s) of a given object. ''' user_obj = get_user(session, user) @@ -706,68 +724,85 @@ def remove_tags_obj( return 'Removed tag: %s' % ', '.join(removed_tags) -def edit_issue_tags(session, project, old_tag, new_tag, ticketfolder, user): +def edit_issue_tags(session, project, old_tag, new_tag, old_tag_color, + new_tag_color, ticketfolder, user): ''' Removes the specified tag of a project. ''' user_obj = get_user(session, user) - if old_tag == new_tag: + if old_tag == new_tag and old_tag_color == new_tag_color: + # check for change raise pagure.exceptions.PagureException( - 'Old tag: "%s" is the same as new tag "%s", nothing to change' - % (old_tag, new_tag)) + 'No change. Old tag "%s(%s)" is the same as new tag "%s(%s)"' + % (old_tag, old_tag_color, new_tag, new_tag_color)) + elif old_tag != new_tag: + # Check if new tag already exists + existing_tag = session.query(model.Tag).filter( + model.Tag.tag == new_tag).filter( + model.Tag.project_id == project.id).all() + if existing_tag is not None and len(existing_tag) > 0: + raise pagure.exceptions.PagureException( + 'Can not rename a tag to an existing tag name: ' + new_tag) issues = search_issues(session, project, closed=False, tags=old_tag) issues.extend(search_issues(session, project, closed=True, tags=old_tag)) - msgs = [] - if not issues: - raise pagure.exceptions.PagureException( - 'No issue found with the tags: %s' % old_tag) - else: - tagobj = get_tag(session, new_tag) - if not tagobj: - tagobj = model.Tag(tag=new_tag) - session.add(tagobj) - session.flush() + # Grab the old tag (there can only be one), and remove it. + orig_tags = session.query(model.Tag).filter( + model.Tag.tag == old_tag).filter( + model.Tag.project_id == project.id).all() + if orig_tags is not None and len(orig_tags) > 0: + session.delete(orig_tags[0]) + session.commit() # flush + + # Now, add the new tag since the old was removed + tagobj = model.Tag(tag=new_tag, tag_color=new_tag_color, + project_id=project.id) + session.add(tagobj) + session.flush() + # Update the Issues tag list for issue in set(issues): - add = True - # Drop the old tag + # Drop the old tag from the issue cnt = 0 while cnt < len(issue.tags): issue_tag = issue.tags[cnt] if issue_tag.tag == old_tag: issue.tags.remove(issue_tag) cnt -= 1 - if issue_tag.tag == new_tag: - add = False cnt += 1 session.flush() - # Add the new one - if add: - issue_tag = model.TagIssue( - issue_uid=issue.uid, - tag=tagobj.tag - ) - session.add(issue_tag) - session.flush() + # Add the new one to the issue + issue_tag = model.TagIssue( + issue_uid=issue.uid, + tag=tagobj.tag, + tag_color=tagobj.tag_color + ) + issue.tags.append(issue_tag) + session.add(issue_tag) + session.flush() # Update the git version pagure.lib.git.update_git( issue, repo=issue.project, repofolder=ticketfolder) + else: + raise pagure.exceptions.PagureException( + 'Tag not found: %s' % old_tag) - msgs.append('Edited tag: %s to %s' % (old_tag, new_tag)) - pagure.lib.notify.log( - project, - topic='project.tag.edited', - msg=dict( - project=project.to_json(public=True), - old_tag=old_tag, - new_tag=new_tag, - agent=user_obj.username, - ), - redis=REDIS, - ) + msgs = [] + msgs.append('Edited tag: %s(%s) to %s(%s)' % + (old_tag, old_tag_color, new_tag, new_tag_color)) + pagure.lib.notify.log( + project, + topic='project.tag.edited', + msg=dict( + project=project.to_json(public=True), + old_tag=old_tag, + new_tag=new_tag, + agent=user_obj.username, + ), + redis=REDIS, + ) return msgs @@ -2014,11 +2049,9 @@ def get_tags_of_project(session, project, pattern=None): query = session.query( model.Tag ).filter( - model.Tag.tag == model.TagIssue.tag - ).filter( - model.TagIssue.issue_uid == model.Issue.uid + model.Tag.tag != "" ).filter( - model.Issue.project_id == project.id + model.Tag.project_id == project.id ).order_by( model.Tag.tag ) diff --git a/pagure/lib/model.py b/pagure/lib/model.py index f7cfbac..43cfed8 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -1021,9 +1021,19 @@ class Tag(BASE): __tablename__ = 'tags' - tag = sa.Column(sa.String(255), primary_key=True) + tag_id = sa.Column(sa.String(255), primary_key=True) + tag = sa.Column(sa.String(255), nullable=False) + tag_color = sa.Column(sa.String(25), default="DeepSkyBlue") date_created = sa.Column(sa.DateTime, nullable=False, default=datetime.datetime.utcnow) + project_id = sa.Column(sa.Integer, nullable=False) + + def __init__(self, tag=None, project_id=None, tag_color="DeepSkyBlue"): + # Create our unique tag identifier + self.tag_id = tag + "_" + str(project_id) + self.tag = tag + self.project_id = project_id + self.tag_color = tag_color class TagIssue(BASE): @@ -1040,6 +1050,11 @@ class TagIssue(BASE): 'tags.tag', ondelete='CASCADE', onupdate='CASCADE', ), primary_key=True) + tag_color = sa.Column( + sa.String(25), + sa.ForeignKey( + 'tags.tag_color', ondelete='CASCADE', onupdate='CASCADE', + )) issue_uid = sa.Column( sa.String(32), sa.ForeignKey( @@ -1056,7 +1071,7 @@ class TagIssue(BASE): ) def __repr__(self): - return 'TagIssue(issue:%s, tag:%s)' % (self.issue.id, self.tag) + return ('TagIssue(issue:%s, tag:%s)' % (self.issue.id, self.tag)) class TagProject(BASE): diff --git a/pagure/templates/edit_tag.html b/pagure/templates/edit_tag.html index 254fbcf..6c17756 100644 --- a/pagure/templates/edit_tag.html +++ b/pagure/templates/edit_tag.html @@ -8,25 +8,93 @@ {% block repo %} -

Edit tag: {{ edit_tag }}

-
-
- -

Enter in the field below the new name for the tag: "{{ edit_tag }}"

- - {{ render_field_in_row(form.tag) }} -
-

- - - {{ form.csrf_token }} -

-
+
+
+
+ Edit tag: {{ edit_tag }} +
+
+ {{ form.csrf_token }} +
+
+ +
+
+ +
+
+

+ + + {{ form.csrf_token }} +

+
+
+
- {% endblock %} diff --git a/pagure/templates/issue.html b/pagure/templates/issue.html index 0e8ac65..45d0b7f 100644 --- a/pagure/templates/issue.html +++ b/pagure/templates/issue.html @@ -159,7 +159,7 @@

{% for tag in issue.tags %} -
- {% for tag in tags %} + {% for tag in tag_list %} {% endfor %} {% endif %} - {% for tag in issue.tags%} - {{tag.tag}} + {% for tag in issue.tags %} + {{tag.tag}} {% endfor%} diff --git a/pagure/templates/settings.html b/pagure/templates/settings.html index db46a0c..6996f58 100644 --- a/pagure/templates/settings.html +++ b/pagure/templates/settings.html @@ -743,8 +743,7 @@

- Here below is the list of tags associated with one or more issue of - the project. + Here is the list of tags associated with this project.

+ + {{ tag_form.csrf_token }} +
+ +
+
+
+
+ + Add New Tag + +
+
+
+
+ +
+
+
+ {% endif %} @@ -944,6 +973,49 @@ $('#default_priorities').click(function(e) { }); {% endif %} + +var first_new_tag = 1; +$('#new_tag').click(function(e) { + console.log('new tag'); + console.log($('#tagcolor')); + if (first_new_tag == 1){ + // Only display the Tag row the first time Add New Tag is clicked + $('#tagcolor').append( + '
\ +
\ + New Tag\ +
\ +
\ + Tag Color\ +
\ +
'); + first_new_tag = 0; + } + $('#tagcolor').append( + '
\ +
\ + \ +
\ +
\ + \ +
\ +
' + ); +}); + $('.extend-form').click(function(e) { const tgt = $(this).attr('data-target'); let form = $(tgt + ' > div:last-child').clone(); diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index 6b104e9..c186613 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -32,6 +32,7 @@ import pagure.exceptions import pagure.lib import pagure.lib.encoding_utils import pagure.forms +from pagure.lib import model from pagure import (APP, SESSION, LOG, __get_file_in_tree, login_required, authenticated) @@ -410,16 +411,20 @@ def edit_tag(repo, tag, username=None, namespace=None): flask.abort(404, 'No issue tracker found for this project') tags = pagure.lib.get_tags_of_project(SESSION, repo) - if not tags or tag not in [t.tag for t in tags]: flask.abort(404, 'Tag %s not found in this project' % tag) + # Get the old color + for t in tags: + if t.tag == tag: + old_tag_color = t.tag_color + form = pagure.forms.AddIssueTagForm() if form.validate_on_submit(): new_tag = form.tag.data - + new_tag_color = form.tag_color.data msgs = pagure.lib.edit_issue_tags( - SESSION, repo, tag, new_tag, + SESSION, repo, tag, new_tag, old_tag_color, new_tag_color, user=flask.g.fas_user.username, ticketfolder=APP.config['TICKETS_FOLDER'] ) @@ -443,9 +448,76 @@ def edit_tag(repo, tag, username=None, namespace=None): username=username, repo=repo, edit_tag=tag, + tag_color=old_tag_color ) +@APP.route('//update/tags', methods=['POST']) +@APP.route('///update/tags', methods=['POST']) +@login_required +def update_tags(repo, username=None, namespace=None): + """ Update the tags of a project. + """ + + repo = flask.g.repo + + if not repo.settings.get('issue_tracker', True): + flask.abort(404, 'No issue tracker found for this project') + + if not flask.g.repo_admin: + flask.abort( + 403, + 'You are not allowed to change the settings for this project') + + form = pagure.forms.ConfirmationForm() + + error = False + if form.validate_on_submit(): + tag_names = flask.request.form.getlist('tag') + tag_colors = flask.request.form.getlist('tag_color_select') + + tags = [] + colors = [] + for t in range(len(tag_names)): + if tag_names[t] == "": + # Blank field, ignore + continue + tags.append(tag_names[t].strip()) + colors.append(tag_colors[t].strip()) + + if len(tags) != len(colors): + flask.flash( + 'tags and tag colors are not of the same length', 'error') + error = True + + for tag in tags: + if tag.strip() and tags.count(tag) != 1: + flask.flash( + 'Tag %s is present %s times' % ( + tag, tags.count(tag) + ), + 'error') + error = True + break + + if not error: + for cnt in range(len(tags)): + new_tag = model.Tag(tag=tags[cnt].strip(), + tag_color=colors[cnt], + project_id=repo.id) + try: + SESSION.add(new_tag) + SESSION.commit() + flask.flash('Tags updated') + except SQLAlchemyError as err: # pragma: no cover + SESSION.rollback() + flask.flash(str(err), 'error') + + return flask.redirect(flask.url_for( + 'view_settings', username=username, repo=repo.name, + namespace=namespace)) + + @APP.route('//droptag/', methods=['POST']) @APP.route('///droptag/', methods=['POST']) @APP.route('/fork///droptag/', methods=['POST']) diff --git a/tests/test_pagure_flask_api.py b/tests/test_pagure_flask_api.py index 44fffca..4f7c750 100644 --- a/tests/test_pagure_flask_api.py +++ b/tests/test_pagure_flask_api.py @@ -77,7 +77,7 @@ class PagureFlaskApitests(tests.Modeltests): self.session.add(item) self.session.commit() item = pagure.lib.model.Tag( - tag='tag1', + tag='tag1', tag_color='DeepBlueSky', project_id=1, ) self.session.add(item) self.session.commit() diff --git a/tests/test_pagure_flask_ui_issues.py b/tests/test_pagure_flask_ui_issues.py index 2be6050..5348683 100644 --- a/tests/test_pagure_flask_ui_issues.py +++ b/tests/test_pagure_flask_ui_issues.py @@ -1512,31 +1512,26 @@ class PagureFlaskIssuestests(tests.Modeltests): output = self.app.get('/test/tag/tag1/edit') self.assertEqual(output.status_code, 200) - self.assertTrue('

Edit tag: tag1

' in output.data) - self.assertTrue( - '

Enter in the field below the new name for the tag: ' - '"tag1"

' in output.data) + self.assertTrue('Edit tag: tag1' in output.data) csrf_token = output.data.split( 'name="csrf_token" type="hidden" value="')[1].split('">')[0] - data = {'tag': 'tag2'} + data = {'tag': 'tag2', + 'tag_color': 'DeepSkyBlue'} output = self.app.post('/test/tag/tag1/edit', data=data) self.assertEqual(output.status_code, 200) - self.assertTrue('

Edit tag: tag1

' in output.data) - self.assertTrue( - '

Enter in the field below the new name for the tag: ' - '"tag1"

' in output.data) + self.assertTrue('Edit tag: tag1' in output.data) data['csrf_token'] = csrf_token output = self.app.post( '/test/tag/tag1/edit', data=data, follow_redirects=True) self.assertEqual(output.status_code, 200) self.assertIn( - 'Settings - test - Pagure', output.data) + 'Settings - test - Pagure', output.data) self.assertIn( - '\n Edited tag: tag1 to tag2', + '\n Edited tag: tag1(DeepSkyBlue) to tag2(DeepSkyBlue)', output.data) # After edit, list tags diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 6432e7f..9d1d7dc 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -365,7 +365,6 @@ class PagureLibtests(tests.Modeltests): self.test_add_tag_obj() repo = pagure.lib.get_project(self.session, 'test') issue = pagure.lib.search_issues(self.session, repo, issueid=1) - self.assertRaises( pagure.exceptions.PagureException, pagure.lib.remove_tags, @@ -421,7 +420,9 @@ class PagureLibtests(tests.Modeltests): session=self.session, project=repo, old_tag='foo', + old_tag_color='DeepSkyBlue', new_tag='bar', + new_tag_color='black', user='pingou', ticketfolder=None, ) @@ -430,12 +431,14 @@ class PagureLibtests(tests.Modeltests): session=self.session, project=repo, old_tag='tag1', + old_tag_color='DeepSkyBlue', new_tag='tag2', + new_tag_color='black', user='pingou', ticketfolder=None, ) self.session.commit() - self.assertEqual(msgs, ['Edited tag: tag1 to tag2']) + self.assertEqual(msgs, ['Edited tag: tag1(DeepSkyBlue) to tag2(black)']) # Add a new tag msg = pagure.lib.add_tag_obj( @@ -448,26 +451,16 @@ class PagureLibtests(tests.Modeltests): self.assertEqual(msg, 'Tag added: tag3') self.assertEqual([tag.tag for tag in issue.tags], ['tag2', 'tag3']) - # Rename an existing tag into another existing one - msgs = pagure.lib.edit_issue_tags( - session=self.session, - project=repo, - old_tag='tag2', - new_tag='tag3', - user='pingou', - ticketfolder=None, - ) - self.session.commit() - self.assertEqual(msgs, ['Edited tag: tag2 to tag3']) - self.assertEqual([tag.tag for tag in issue.tags], ['tag3']) - + # Attempt to rename an existing tag into another existing one self.assertRaises( pagure.exceptions.PagureException, pagure.lib.edit_issue_tags, session=self.session, project=repo, old_tag='tag2', - new_tag='tag2', + old_tag_color='black', + new_tag='tag3', + new_tag_color='red', user='pingou', ticketfolder=None, ) diff --git a/tests/test_pagure_lib_model.py b/tests/test_pagure_lib_model.py index 7457430..56bfb00 100644 --- a/tests/test_pagure_lib_model.py +++ b/tests/test_pagure_lib_model.py @@ -132,17 +132,18 @@ class PagureLibModeltests(tests.Modeltests): issues = pagure.lib.search_issues(self.session, repo) self.assertEqual(len(issues), 1) - item = pagure.lib.model.Tag(tag='foo') + item = pagure.lib.model.Tag(tag='foo', tag_color='DeepSkyBlue', + project_id=repo.id) self.session.add(item) self.session.commit() item = pagure.lib.model.TagIssue( + issue=issues[0], issue_uid=issues[0].uid, - tag='foo', + tag='foo' ) self.session.add(item) self.session.commit() - self.assertEqual(str(item), 'TagIssue(issue:1, tag:foo)') From 39c2bdc3ad3413afdffacbcf2620337429bfe3dd Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Dec 09 2016 15:55:02 +0000 Subject: [PATCH 2/4] Applied recommended changes - Removed nbsp's and just set the width for the dropdown - Added lib function to create new tag - Consolidated color selector code in a for loop --- diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 3bff234..cc78361 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -1366,6 +1366,11 @@ def new_pull_request(session, branch_from, return request +def new_tag(tag_name, tag_color, project_id): + ''' Return a new tag object ''' + return model.Tag(tag=tag_name, tag_color=tag_color, project_id=project_id) + + def edit_issue(session, issue, ticketfolder, user, title=None, content=None, status=None, close_status=None, priority=None, milestone=None, private=False): diff --git a/pagure/templates/edit_tag.html b/pagure/templates/edit_tag.html index 6c17756..69bc8d4 100644 --- a/pagure/templates/edit_tag.html +++ b/pagure/templates/edit_tag.html @@ -28,63 +28,13 @@ value={{ edit_tag }} size="3" class="form-control"/>
-
diff --git a/pagure/templates/settings.html b/pagure/templates/settings.html index 6996f58..0dc3c66 100644 --- a/pagure/templates/settings.html +++ b/pagure/templates/settings.html @@ -998,8 +998,9 @@ $('#new_tag').click(function(e) { value="" size="3" class="form-control"/>\ \
\ - \ + \ \ \ \ diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index c186613..fb7daf9 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -32,7 +32,6 @@ import pagure.exceptions import pagure.lib import pagure.lib.encoding_utils import pagure.forms -from pagure.lib import model from pagure import (APP, SESSION, LOG, __get_file_in_tree, login_required, authenticated) @@ -502,9 +501,8 @@ def update_tags(repo, username=None, namespace=None): if not error: for cnt in range(len(tags)): - new_tag = model.Tag(tag=tags[cnt].strip(), - tag_color=colors[cnt], - project_id=repo.id) + new_tag = pagure.lib.new_tag(tags[cnt].strip(), + colors[cnt], repo.id) try: SESSION.add(new_tag) SESSION.commit() From 73238fa8379527213d6a134e102fd9e2781445c4 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Dec 10 2016 00:29:46 +0000 Subject: [PATCH 3/4] Applied latest recommendations (part 2) --- diff --git a/alembic/versions/2c81d9228a35_add_tag_color_to_tags_table.py b/alembic/versions/2c81d9228a35_add_tag_color_to_tags_table.py deleted file mode 100644 index 026bb45..0000000 --- a/alembic/versions/2c81d9228a35_add_tag_color_to_tags_table.py +++ /dev/null @@ -1,51 +0,0 @@ -"""add tag_color to tags table - -Revision ID: 2c81d9228a35 -Revises: 5083efccac7 -Create Date: 2016-11-21 15:22:09.793435 - -""" - -# revision identifiers, used by Alembic. -revision = '2c81d9228a35' -down_revision = '5083efccac7' - -from alembic import op -import sqlalchemy as sa - - -def upgrade(): - ''' Add the column "tag_color" to the table "tags". - ''' - op.add_column( - 'tag_id', - sa.Column('tag_id', sa.String(25), primary_key=True) - ) - op.add_column( - 'tag_color', - sa.Column('tag_color', sa.String(25), default="DeepSkyBlue") - ) - op.add_column( - 'project_id', - sa.Column(sa.Integer, nullable=False) - ) - - ''' Need to update/replace every tag with the new parameters tag_id, - project_id, and tag_color. Multiple projects could be sharing the same tag - name. So we might need to create many new tags of the same name, but - different tag_id's. - - The tag ID (primary_key) is the tag name plus the project id: - - _ = myNewTag_27 - - So we need to check all the TagIssues, find its project id's, and create a - new tag (with the tag_id as the primary key). Then we need to remove the - old Tags. Easy, right? - ''' - - -def downgrade(): - ''' Remove the column "tag_color" from the table "tags". - ''' - op.drop_column('tag_id', 'tags', 'tag_color', 'project_id') diff --git a/pagure/default_config.py b/pagure/default_config.py index edb8302..14d180b 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -227,3 +227,7 @@ BOOTSTRAP_URLS_JS = 'https://apps.fedoraproject.org/global/' \ # List of the type of CI service supported by this pagure instance PAGURE_CI_SERVICES = [] + +# list of allowed tag colors +TAG_COLOR_LIST = ['DeepSkyBlue', 'red', 'maroon', 'SlateBlue', 'teal', 'green', + 'brown', 'orange', 'gray', 'purple', 'black'] \ No newline at end of file diff --git a/pagure/forms.py b/pagure/forms.py index acc89dd..d38f104 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -34,8 +34,6 @@ STRICT_REGEX = '^[a-zA-Z0-9-_]+$' TAGS_REGEX = '^[a-zA-Z0-9-_, .]+$' PROJECT_NAME_REGEX = \ '^[a-zA-z0-9_][a-zA-Z0-9-_]*$' -TAG_COLOR_LIST = ['DeepSkyBlue', 'red', 'maroon', 'SlateBlue', 'teal', 'green', - 'brown', 'orange', 'gray', 'purple', 'black'] class PagureForm(FlaskForm): @@ -245,7 +243,8 @@ class AddIssueTagForm(PagureForm): def __init__(self, *args, **kwargs): super(AddIssueTagForm, self).__init__(*args, **kwargs) self.tag_color.choices = [ - (tag_color, tag_color) for tag_color in TAG_COLOR_LIST + (tag_color, tag_color) for tag_color in + pagure.APP.config['TAG_COLOR_LIST'] ] diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index cc78361..457a8b1 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -329,7 +329,11 @@ def add_tag_obj(session, obj, tags, user, ticketfolder): if known: continue - tagobj = get_tag(session, objtag) + if isinstance(obj, model.Project): + proj_id = obj.id + else: + proj_id = obj.project.id + tagobj = get_tag(session, objtag, proj_id) if not tagobj: if isinstance(obj, model.Project): project_id = obj.id @@ -347,7 +351,8 @@ def add_tag_obj(session, obj, tags, user, ticketfolder): dbobjtag = model.TagIssue( issue_uid=obj.uid, tag=tagobj.tag, - tag_color=tagobj.tag_color + tag_color=tagobj.tag_color, + tag_id=tagobj.tag_id ) if isinstance(obj, model.Project): dbobjtag = model.TagProject( @@ -750,9 +755,10 @@ def edit_issue_tags(session, project, old_tag, new_tag, old_tag_color, orig_tags = session.query(model.Tag).filter( model.Tag.tag == old_tag).filter( model.Tag.project_id == project.id).all() + if orig_tags is not None and len(orig_tags) > 0: session.delete(orig_tags[0]) - session.commit() # flush + session.commit() # Now, add the new tag since the old was removed tagobj = model.Tag(tag=new_tag, tag_color=new_tag_color, @@ -776,7 +782,8 @@ def edit_issue_tags(session, project, old_tag, new_tag, old_tag_color, issue_tag = model.TagIssue( issue_uid=issue.uid, tag=tagobj.tag, - tag_color=tagobj.tag_color + tag_color=tagobj.tag_color, + tag_id=tagobj.tag_id ) issue.tags.append(issue_tag) session.add(issue_tag) @@ -798,7 +805,9 @@ def edit_issue_tags(session, project, old_tag, new_tag, old_tag_color, msg=dict( project=project.to_json(public=True), old_tag=old_tag, + old_tag_color=old_tag_color, new_tag=new_tag, + new_tag_color=new_tag_color, agent=user_obj.username, ), redis=REDIS, @@ -2069,13 +2078,15 @@ def get_tags_of_project(session, project, pattern=None): return query.all() -def get_tag(session, tag): +def get_tag(session, tag, project_id): ''' Returns a Tag object for the given tag text. ''' query = session.query( model.Tag ).filter( model.Tag.tag == tag + ).filter( + model.Tag.project_id == project_id ) return query.first() diff --git a/pagure/lib/model.py b/pagure/lib/model.py index 43cfed8..e596b76 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -1021,7 +1021,7 @@ class Tag(BASE): __tablename__ = 'tags' - tag_id = sa.Column(sa.String(255), primary_key=True) + tag_id = sa.Column(sa.Integer, primary_key=True) tag = sa.Column(sa.String(255), nullable=False) tag_color = sa.Column(sa.String(25), default="DeepSkyBlue") date_created = sa.Column(sa.DateTime, nullable=False, @@ -1030,7 +1030,6 @@ class Tag(BASE): def __init__(self, tag=None, project_id=None, tag_color="DeepSkyBlue"): # Create our unique tag identifier - self.tag_id = tag + "_" + str(project_id) self.tag = tag self.project_id = project_id self.tag_color = tag_color @@ -1044,23 +1043,28 @@ class TagIssue(BASE): __tablename__ = 'tags_issues' - tag = sa.Column( - sa.String(255), + tag_id = sa.Column( + sa.Integer, sa.ForeignKey( - 'tags.tag', ondelete='CASCADE', onupdate='CASCADE', + 'tags.tag_id', ondelete='CASCADE', onupdate='CASCADE', ), primary_key=True) - tag_color = sa.Column( - sa.String(25), - sa.ForeignKey( - 'tags.tag_color', ondelete='CASCADE', onupdate='CASCADE', - )) issue_uid = sa.Column( sa.String(32), sa.ForeignKey( 'issues.uid', ondelete='CASCADE', onupdate='CASCADE', ), primary_key=True) + tag_color = sa.Column( + sa.String(25), + sa.ForeignKey( + 'tags.tag_color', ondelete='CASCADE', onupdate='CASCADE', + )) + tag = sa.Column( + sa.String(255), + sa.ForeignKey( + 'tags.tag', ondelete='CASCADE', onupdate='CASCADE', + )) date_created = sa.Column(sa.DateTime, nullable=False, default=datetime.datetime.utcnow) @@ -1071,7 +1075,7 @@ class TagIssue(BASE): ) def __repr__(self): - return ('TagIssue(issue:%s, tag:%s)' % (self.issue.id, self.tag)) + return 'TagIssue(issue:%s, tag:%s)' % (self.issue.id, self.tag) class TagProject(BASE): diff --git a/pagure/templates/edit_tag.html b/pagure/templates/edit_tag.html index 69bc8d4..0cc503a 100644 --- a/pagure/templates/edit_tag.html +++ b/pagure/templates/edit_tag.html @@ -30,10 +30,8 @@
diff --git a/pagure/templates/settings.html b/pagure/templates/settings.html index 0dc3c66..2b65ce5 100644 --- a/pagure/templates/settings.html +++ b/pagure/templates/settings.html @@ -1000,17 +1000,9 @@ $('#new_tag').click(function(e) {
\ \
\
' diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index fb7daf9..ee468fa 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -248,7 +248,6 @@ def update_issue(repo, issueid, username=None, namespace=None): SESSION.commit() if message and not is_js: messages.add(message) - if repo_admin: # Adjust (add/remove) tags messages.union(set(pagure.lib.update_tags( @@ -410,13 +409,18 @@ def edit_tag(repo, tag, username=None, namespace=None): flask.abort(404, 'No issue tracker found for this project') tags = pagure.lib.get_tags_of_project(SESSION, repo) - if not tags or tag not in [t.tag for t in tags]: - flask.abort(404, 'Tag %s not found in this project' % tag) + if not tags: + flask.abort(404, 'Project has no tags to edit') - # Get the old color + # Check the tag exists, and get its old/original color + found = False for t in tags: if t.tag == tag: old_tag_color = t.tag_color + found = True + break + if not found: + flask.abort(404, 'Tag %s not found in this project' % tag) form = pagure.forms.AddIssueTagForm() if form.validate_on_submit(): @@ -447,7 +451,8 @@ def edit_tag(repo, tag, username=None, namespace=None): username=username, repo=repo, edit_tag=tag, - tag_color=old_tag_color + tag_color=old_tag_color, + color_list=pagure.APP.config['TAG_COLOR_LIST'] ) diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 096aa97..084fa98 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -1051,6 +1051,7 @@ def view_settings(repo, username=None, namespace=None): tags=tags, plugins=plugins, branchname=branchname, + color_list=pagure.APP.config['TAG_COLOR_LIST'] ) diff --git a/tests/test_pagure_flask_api.py b/tests/test_pagure_flask_api.py index 4f7c750..cc2269b 100644 --- a/tests/test_pagure_flask_api.py +++ b/tests/test_pagure_flask_api.py @@ -84,6 +84,7 @@ class PagureFlaskApitests(tests.Modeltests): item = pagure.lib.model.TagIssue( tag='tag1', issue_uid='foobar', + tag_id=tag.tag_id ) self.session.add(item) self.session.commit() diff --git a/tests/test_pagure_lib.py b/tests/test_pagure_lib.py index 9d1d7dc..6ed8439 100644 --- a/tests/test_pagure_lib.py +++ b/tests/test_pagure_lib.py @@ -465,6 +465,20 @@ class PagureLibtests(tests.Modeltests): ticketfolder=None, ) + # Rename an existing tag + msgs = pagure.lib.edit_issue_tags( + session=self.session, + project=repo, + old_tag='tag2', + old_tag_color='black', + new_tag='tag4', + new_tag_color='purple', + user='pingou', + ticketfolder=None, + ) + self.session.commit() + self.assertEqual(msgs, ['Edited tag: tag2(black) to tag4(purple)']) + @patch('pagure.lib.git.update_git') @patch('pagure.lib.notify.send_email') def test_search_issues(self, p_send_email, p_ugt): diff --git a/tests/test_pagure_lib_model.py b/tests/test_pagure_lib_model.py index 56bfb00..9708b4a 100644 --- a/tests/test_pagure_lib_model.py +++ b/tests/test_pagure_lib_model.py @@ -140,7 +140,8 @@ class PagureLibModeltests(tests.Modeltests): item = pagure.lib.model.TagIssue( issue=issues[0], issue_uid=issues[0].uid, - tag='foo' + tag='foo', + tag_id=item.tag_id ) self.session.add(item) self.session.commit() From c8afc372324be9911873ebb4ac0a35956714a992 Mon Sep 17 00:00:00 2001 From: Mark Reynolds Date: Dec 10 2016 00:40:11 +0000 Subject: [PATCH 4/4] Merge branch 'issue1544' of ssh://pagure.io/forks/mreynolds/pagure into issue1544 --- diff --git a/pagure/forms.py b/pagure/forms.py index d38f104..3686fc9 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -34,6 +34,8 @@ STRICT_REGEX = '^[a-zA-Z0-9-_]+$' TAGS_REGEX = '^[a-zA-Z0-9-_, .]+$' PROJECT_NAME_REGEX = \ '^[a-zA-z0-9_][a-zA-Z0-9-_]*$' +TAG_COLOR_LIST = ['DeepSkyBlue', 'red', 'maroon', 'SlateBlue', 'teal', 'green', + 'brown', 'orange', 'gray', 'purple', 'black'] class PagureForm(FlaskForm): diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 457a8b1..919bd55 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -755,7 +755,6 @@ def edit_issue_tags(session, project, old_tag, new_tag, old_tag_color, orig_tags = session.query(model.Tag).filter( model.Tag.tag == old_tag).filter( model.Tag.project_id == project.id).all() - if orig_tags is not None and len(orig_tags) > 0: session.delete(orig_tags[0]) session.commit() diff --git a/pagure/lib/model.py b/pagure/lib/model.py index e596b76..95166d9 100644 --- a/pagure/lib/model.py +++ b/pagure/lib/model.py @@ -1049,6 +1049,11 @@ class TagIssue(BASE): 'tags.tag_id', ondelete='CASCADE', onupdate='CASCADE', ), primary_key=True) + tag_color = sa.Column( + sa.String(25), + sa.ForeignKey( + 'tags.tag_color', ondelete='CASCADE', onupdate='CASCADE', + )) issue_uid = sa.Column( sa.String(32), sa.ForeignKey( @@ -1075,7 +1080,7 @@ class TagIssue(BASE): ) def __repr__(self): - return 'TagIssue(issue:%s, tag:%s)' % (self.issue.id, self.tag) + return ('TagIssue(issue:%s, tag:%s)' % (self.issue.id, self.tag)) class TagProject(BASE): diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index ee468fa..bc01f78 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -422,6 +422,11 @@ def edit_tag(repo, tag, username=None, namespace=None): if not found: flask.abort(404, 'Tag %s not found in this project' % tag) + # Get the old color + for t in tags: + if t.tag == tag: + old_tag_color = t.tag_color + form = pagure.forms.AddIssueTagForm() if form.validate_on_submit(): new_tag = form.tag.data