From 3b03480f827992e652a2aaaae9046f14ca5f0ef8 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 28 2016 08:55:56 +0000 Subject: Rework the colored tag feature This commit does a few thing: - Drop the hard-coded list of color - Instead of this list, offer the user to pick any color they wishes to use in a color-picker widget (html5 powered) - Fix editing a tag - simplify the logic while at it so that it runs an update query instead of having to manipulate the issues (removing the old tag then adding the new one...) - Fix updating a tag - Fix deleting a tag - Small code clean here and there to avoid duplication (most around the using the get_tag() method) - Adjust pagure.lib for the change in the DB model to use TagColored where we were using Tag --- diff --git a/pagure/default_config.py b/pagure/default_config.py index 7fb7cfd..4dc234e 100644 --- a/pagure/default_config.py +++ b/pagure/default_config.py @@ -195,7 +195,7 @@ APPLICATION_ROOT = '/' BLACKLISTED_PROJECTS = [ 'static', 'pv', 'releases', 'new', 'api', 'settings', 'search', 'fork', 'logout', 'login', 'user', 'users', 'groups', 'projects', 'ssh_info', - 'issues', 'pull-requests', 'commits', 'tree', 'forks', 'admin', + 'issues', 'pull-requests', 'commits', 'tree', 'forks', 'admin', ] # List of prefix allowed in project names @@ -232,7 +232,3 @@ PAGURE_CI_SERVICES = [] # Boolean to turn on project being by default in the user's namespace USER_NAMESPACE = False - -# list of allowed tag colors -TAG_COLOR_LIST = ['DeepSkyBlue', 'red', 'maroon', 'SlateBlue', 'teal', 'green', - 'brown', 'orange', 'gray', 'purple', 'black'] diff --git a/pagure/forms.py b/pagure/forms.py index 1283802..576896f 100644 --- a/pagure/forms.py +++ b/pagure/forms.py @@ -225,8 +225,8 @@ class RemoteRequestPullForm(RequestPullForm): ) -class AddIssueTagForm(PagureForm): - ''' Form to add a tag to an issue. ''' +class DeleteIssueTagForm(PagureForm): + ''' Form to remove a tag to from a project. ''' tag = wtforms.TextField( 'tag', [ @@ -235,19 +235,15 @@ class AddIssueTagForm(PagureForm): wtforms.validators.Length(max=255), ] ) - tag_color = wtforms.SelectField( + + +class AddIssueTagForm(DeleteIssueTagForm): + ''' Form to add a tag to a project. ''' + tag_color = wtforms.TextField( 'tag_color', - [wtforms.validators.Optional()], - choices=[] + [wtforms.validators.Required()], ) - def __init__(self, *args, **kwargs): - super(AddIssueTagForm, self).__init__(*args, **kwargs) - self.tag_color.choices = [ - (tag_color, tag_color) for tag_color in - pagure.APP.config['TAG_COLOR_LIST'] - ] - class StatusForm(PagureForm): ''' Form to add/change the status of an issue. ''' diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py index 3498a2f..222c334 100644 --- a/pagure/lib/__init__.py +++ b/pagure/lib/__init__.py @@ -336,32 +336,33 @@ def add_tag_obj(session, obj, tags, user, ticketfolder): 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 - else: - project_id = obj.project.id - tagobj = model.Tag(tag=objtag, - tag_color="DeepSkyBlue", - project_id=project_id) + if obj.isa == 'project': + if not tagobj: + tagobj = model.Tag(tag=objtag) - session.add(tagobj) - session.flush() + session.add(tagobj) + session.flush() - if isinstance(obj, model.Issue): - dbobjtag = model.TagIssue( - issue_uid=obj.uid, - tag=tagobj.tag, - tag_color=tagobj.tag_color, - tag_id=tagobj.tag_id - ) - if isinstance(obj, model.Project): dbobjtag = model.TagProject( project_id=obj.id, tag=tagobj.tag, ) + else: + if not tagobj: + tagobj = model.TagColored( + tag=objtag, + project_id=proj_id + ) + session.add(tagobj) + session.flush() + + dbobjtag = model.TagIssueColored( + issue_uid=obj.uid, + tag_id=tagobj.id + ) + session.add(dbobjtag) # Make sure we won't have SQLAlchemy error before we continue session.flush() @@ -657,14 +658,12 @@ def remove_tags(session, project, tags, ticketfolder, user): removed_tags = [] 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: + tagobj = get_tag(session, tag, project.id) + if tagobj: tag_found = True removed_tags.append(tag) msgs.append('Removed tag: %s' % tag) - session.delete(deltag) + session.delete(tagobj) if not tag_found: raise pagure.exceptions.PagureException( @@ -731,82 +730,68 @@ def remove_tags_obj(session, obj, tags, ticketfolder, user): return 'Removed tag: %s' % ', '.join(removed_tags) -def edit_issue_tags(session, project, old_tag, new_tag, old_tag_color, - new_tag_color, ticketfolder, user): +def edit_issue_tags( + session, project, old_tag, new_tag, + new_tag_color, ticketfolder, user): ''' Removes the specified tag of a project. ''' user_obj = get_user(session, user) + old_tag_name = old_tag - if old_tag == new_tag and old_tag_color == new_tag_color: + if not isinstance(old_tag, model.TagColored): + old_tag = get_tag(session, old_tag_name, project.id) + + if not old_tag: + raise pagure.exceptions.PagureException( + 'No tag "%s" found related to this project' % (old_tag_name)) + + old_tag_name = old_tag.tag + old_tag_color = old_tag.tag_color + if old_tag.tag == new_tag and old_tag_color == new_tag_color: # check for change raise pagure.exceptions.PagureException( '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: + elif old_tag.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: + existing_tag = get_tag(session, new_tag, project.id) + if existing_tag: 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)) - - # 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() - - # 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): - # 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 - cnt += 1 - session.flush() + 'Can not rename a tag to an existing tag name: %s' % new_tag) - # Add the new one to the issue - issue_tag = model.TagIssue( - issue_uid=issue.uid, - tag=tagobj.tag, - tag_color=tagobj.tag_color, - tag_id=tagobj.tag_id - ) - issue.tags.append(issue_tag) - session.add(issue_tag) - session.flush() + session.query( + model.TagColored + ).filter( + model.TagColored.tag == old_tag.tag + ).filter( + model.TagColored.project_id == project.id + ).update( + { + model.TagColored.tag: new_tag, + model.TagColored.tag_color: new_tag_color + } + ) - # 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) + issues = session.query( + model.Issue + ).filter( + model.TagIssueColored.tag_id == old_tag.id + ).filter( + model.TagIssueColored.issue_uid == model.Issue.uid + ).all() + for issue in issues: + # Update the git version + pagure.lib.git.update_git( + issue, repo=issue.project, repofolder=ticketfolder) msgs = [] msgs.append('Edited tag: %s(%s) to %s(%s)' % - (old_tag, old_tag_color, new_tag, new_tag_color)) + (old_tag_name, 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, + old_tag=old_tag.tag, old_tag_color=old_tag_color, new_tag=new_tag, new_tag_color=new_tag_color, @@ -1382,9 +1367,17 @@ def new_pull_request(session, branch_from, return request -def new_tag(tag_name, tag_color, project_id): +def new_tag(session, 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) + tagobj = model.TagColored( + tag=tag_name, + tag_color=tag_color, + project_id=project_id + ) + session.add(tagobj) + session.flush() + + return tagobj def edit_issue(session, issue, ticketfolder, user, @@ -1917,9 +1910,11 @@ def search_issues( ).filter( model.Issue.project_id == repo.id ).filter( - model.Issue.uid == model.TagIssue.issue_uid + model.Issue.uid == model.TagIssueColored.issue_uid ).filter( - model.TagIssue.tag.in_(ytags) + model.TagIssueColored.tag_id == model.TagColored.id + ).filter( + model.TagColored.tag.in_(ytags) ) if notags: sub_q3 = session.query( @@ -1927,9 +1922,11 @@ def search_issues( ).filter( model.Issue.project_id == repo.id ).filter( - model.Issue.uid == model.TagIssue.issue_uid + model.Issue.uid == model.TagIssueColored.issue_uid + ).filter( + model.TagIssueColored.tag_id == model.TagColored.id ).filter( - model.TagIssue.tag.in_(notags) + model.TagColored.tag.in_(notags) ) # Adjust the main query based on the parameters specified if ytags and not notags: @@ -2068,18 +2065,18 @@ def get_tags_of_project(session, project, pattern=None): ''' Returns the list of tags associated with the issues of a project. ''' query = session.query( - model.Tag + model.TagColored ).filter( - model.Tag.tag != "" + model.TagColored.tag != "" ).filter( - model.Tag.project_id == project.id + model.TagColored.project_id == project.id ).order_by( - model.Tag.tag + model.TagColored.tag ) if pattern: query = query.filter( - model.Tag.tag.ilike(pattern.replace('*', '%')) + model.TagColored.tag.ilike(pattern.replace('*', '%')) ) return query.all() @@ -2089,11 +2086,11 @@ def get_tag(session, tag, project_id): ''' Returns a Tag object for the given tag text. ''' query = session.query( - model.Tag + model.TagColored ).filter( - model.Tag.tag == tag + model.TagColored.tag == tag ).filter( - model.Tag.project_id == project_id + model.TagColored.project_id == project_id ) return query.first() diff --git a/pagure/templates/edit_tag.html b/pagure/templates/edit_tag.html index 0cc503a..6775017 100644 --- a/pagure/templates/edit_tag.html +++ b/pagure/templates/edit_tag.html @@ -1,7 +1,7 @@ {% extends "repo_master.html" %} -{% from "_formhelper.html" import render_field_in_row %} +{% from "_formhelper.html" import render_bootstrap_field %} -{% block title %}Edit tag: {{ tag }} - {{ +{% block title %}Edit tag: {{ tagname }} - {{ repo.namespace + '/' if repo.namespace }}{{ repo.name }}{% endblock %} {% set tag = "home" %} @@ -9,40 +9,46 @@ {% block repo %}
-
-
-
- Edit tag: {{ edit_tag }} -
-
+
+
+ Edit tag: {{ tagname }} +
+
+ - {{ form.csrf_token }} -
-
- -
-
- -
-
-

- - + tag=tagname, + namespace=repo.namespace) }}" > {{ form.csrf_token }} -

+ {{ render_bootstrap_field(form.tag) }} +
+ {% set formclasses = "form-control c-select"%} + {% if form.tag_color.errors %} {% set formclasses = formclasses + " form-control-error" %} {% endif %} + + {{ form.tag_color.label }} + + +
+ {% if form.tag_color.errors %} + + + {% for error in form.tag_color.errors %} + {{ error }}  + {% endfor %} + + + {% endif %} +
+
+ +
+
{% endblock %} diff --git a/pagure/templates/settings.html b/pagure/templates/settings.html index 2b65ce5..484d398 100644 --- a/pagure/templates/settings.html +++ b/pagure/templates/settings.html @@ -778,8 +778,7 @@ repo=repo.name, username=username, namespace=repo.namespace, - tag=tag.tag, - tag_color=tag.tag_color) }}"> + tag=tag.tag) }}"> @@ -998,12 +997,8 @@ $('#new_tag').click(function(e) { value="" size="3" class="form-control"/>\ \
\ - \ + \
\ ' ); diff --git a/pagure/ui/issues.py b/pagure/ui/issues.py index 5066277..269ddf5 100644 --- a/pagure/ui/issues.py +++ b/pagure/ui/issues.py @@ -378,21 +378,21 @@ def edit_tag(repo, tag, username=None, namespace=None): flask.abort(404, 'Project has no tags to edit') # 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: + tagobj = pagure.lib.get_tag(SESSION, tag, repo.id) + if not tagobj: flask.abort(404, 'Tag %s not found in this project' % tag) 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, old_tag_color, new_tag_color, + SESSION, + repo, + tagobj, + new_tag, + new_tag_color, user=flask.g.fas_user.username, ticketfolder=APP.config['TICKETS_FOLDER'] ) @@ -409,15 +409,16 @@ def edit_tag(repo, tag, username=None, namespace=None): return flask.redirect(flask.url_for( '.view_settings', repo=repo.name, username=username, namespace=repo.namespace)) + elif flask.request.method == 'GET': + form.tag_color.data = tagobj.tag_color + form.tag.data = tag return flask.render_template( 'edit_tag.html', - form=form, username=username, repo=repo, - edit_tag=tag, - tag_color=old_tag_color, - color_list=pagure.APP.config['TAG_COLOR_LIST'] + form=form, + tagname=tag, ) @@ -443,7 +444,7 @@ def update_tags(repo, username=None, namespace=None): error = False if form.validate_on_submit(): tag_names = flask.request.form.getlist('tag') - tag_colors = flask.request.form.getlist('tag_color_select') + tag_colors = flask.request.form.getlist('tag_color') tags = [] colors = [] @@ -471,10 +472,12 @@ def update_tags(repo, username=None, namespace=None): if not error: for cnt in range(len(tags)): - new_tag = pagure.lib.new_tag(tags[cnt].strip(), - colors[cnt], repo.id) try: - SESSION.add(new_tag) + pagure.lib.new_tag( + SESSION, + tags[cnt].strip(), + colors[cnt], + repo.id) SESSION.commit() flask.flash('Tags updated') except SQLAlchemyError as err: # pragma: no cover @@ -505,7 +508,7 @@ def remove_tag(repo, username=None, namespace=None): if not repo.settings.get('issue_tracker', True): flask.abort(404, 'No issue tracker found for this project') - form = pagure.forms.AddIssueTagForm() + form = pagure.forms.DeleteIssueTagForm() if form.validate_on_submit(): tags = form.tag.data tags = [tag.strip() for tag in tags.split(',')] diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py index 9e80472..60d354c 100644 --- a/pagure/ui/repo.py +++ b/pagure/ui/repo.py @@ -1080,7 +1080,6 @@ def view_settings(repo, username=None, namespace=None): tags=tags, plugins=plugins, branchname=branchname, - color_list=pagure.APP.config['TAG_COLOR_LIST'] )