Modify

Opened 14 years ago

Closed 14 years ago

#7181 closed defect (fixed)

Bugs and Improvement for batch modification for 'keywords' like fields

Reported by: beachroad Owned by: CuriousCurmudgeon
Priority: normal Component: BatchModifyPlugin
Severity: normal Keywords:
Cc: beachroad Trac Release: 0.11

Description

Thanks for your recent changes for keyword modification but it has a bug and it doesn't satisify my requirement very well. So I wrote a short patch as below:

  • Introduce 3 options for customization
    • [batchmod] fields_as_list
    • [batchmod] list_separator_regex
    • [batchmod] list_connecter_string
  • Fix a bug when applying 'keyword'-type modification for multiple tickets.
  • Using Trac 0.11.6, after applying batch modification, I redirect to 'default query' page. I'm not sure but Trac development team change a policy to process requests for '/query' page. I replaced 'actionURL' data by 'query_href' to redirect explicity. (Though actionURL itself didn't work by typo...)
  • batchmod/web_ui.py

     
    33# Copyright (C) 2007 Dave Gynn
    44
    55from trac.core import *
     6from trac.config import Option, ListOption
    67from trac.perm import IPermissionRequestor
    78from trac.ticket import TicketSystem, Ticket
    89from trac.ticket.query import QueryModule
     
    1819class BatchModifyModule(Component):
    1920    implements(IPermissionRequestor, ITemplateProvider, IRequestFilter, ITemplateStreamFilter)
    2021
     22    fields_as_list = ListOption("batchmod", "fields_as_list", default="keywords",
     23                                doc="field names modified as a value list(separated by ',')")
     24    list_separator_regex = Option("batchmod", "list_separator_regex", default='[,\s]+',
     25                                  doc="separator regex used for 'list' fields")
     26    list_connecter_string = Option("batchmod", "list_connector_string", default=',',
     27                                   doc="conneter string for 'list' fields")
     28
    2129    # IPermissionRequestor methods
    2230    def get_permission_actions(self):
    2331        yield 'TICKET_BATCH_MODIFY'
     
    3745            req.args.get('batchmod') and self._has_permission(req):
    3846            self.log.debug('BatchModifyModule: executing')
    3947            self._batch_modify(req)
     48            # redirect to original Query
     49            # (FIMXE: more good way to fake QueryModule...)
     50            req.redirect(req.args.get('query_href'))
     51
    4052        return handler
    4153
    4254    def post_process_request(self, req, template, content_type):
     
    7587                if not modify_changetime:
    7688                  original_changetime = to_timestamp(t.time_changed)
    7789
    78                 if 'keywords' in values:
    79                     values['keywords'] = self._merge_keywords(t.values['keywords'], values['keywords'])
     90                _values = values.copy()
     91                for field in [f for f in values.keys() if f in self.fields_as_list]:
     92                    _values[field] = self._merge_keywords(t.values[field], values[field])
    8093
    81                 t.populate(values)
     94                t.populate(_values)
    8295                t.save_changes(req.authname, comment)
    8396
    8497                if not modify_changetime:
     
    107120        self.log.debug('BatchModifyPlugin: existing keywords are %s', original_keywords)
    108121        self.log.debug('BatchModifyPlugin: new keywords are %s', new_keywords)
    109122
    110         regexp = re.compile(r'[^-\w]+')
     123        regexp = re.compile(self.list_separator_regex)
    111124
    112         new_keywords = set([k.strip() for k in regexp.split(new_keywords) if k])
    113         combined_keywords = set([k.strip() for k in regexp.split(original_keywords) if k])
     125        new_keywords = [k.strip() for k in regexp.split(new_keywords) if k]
     126        combined_keywords = [k.strip() for k in regexp.split(original_keywords) if k]
    114127
    115128        for keyword in new_keywords:
    116129            if keyword.startswith('-'):
    117130                keyword = keyword[1:]
    118                 if keyword in combined_keywords:
     131                while keyword in combined_keywords:
    119132                    combined_keywords.remove(keyword)
    120133            else:
    121                 combined_keywords.add(keyword)
     134                if keyword not in combined_keywords:
     135                    combined_keywords.append(keyword)
    122136
    123137        self.log.debug('BatchModifyPlugin: combined keywords are %s', combined_keywords)
    124         return ', '.join(combined_keywords)
     138        return self.list_connecter_string.join(combined_keywords)
    125139
    126140    # ITemplateStreamFilter methods
    127141    def filter_stream(self, req, method, filename, stream, formdata):
     
    134148
    135149    def _generate_form(self, req, data):
    136150        batchFormData = dict(data)
    137         batchFormData['actionUri']= req.session['query_href'] or req.href.query()
     151        batchFormData['query_href']= req.session['query_href'] or req.href.query()
    138152
    139153        ticketSystem = TicketSystem(self.env)
    140154        fields = []
  • batchmod/templates/batchmod.html

     
    55     xmlns:py="http://genshi.edgewall.org/"
    66     xmlns:xi="http://www.w3.org/2001/XInclude">
    77<body>
    8 <form id="batchmod-form" method="post" action="${actionURI}">
     8<form id="batchmod-form" method="post">
    99<fieldset id="batchmod-fieldset"><legend class="foldable">Batch Modify Properties</legend>
    1010<table class="properties">
    1111     <tr py:for="row in group(fields, 2, lambda f: f.type != 'textarea')"
     
    5353</table>
    5454<div>
    5555    <input type="hidden" name="selectedTickets" value=""/>
     56    <input type="hidden" name="query_href" value="${query_href}"/>
    5657    <input type="submit" id="batchmod" name="batchmod" value="Change tickets" />
    5758    <input type="checkbox" id="bmod_modify_changetime" name="bmod_modify_changetime" value="true" checked="checked" />Modify Ticket Changetime
    5859</div>
     
    105106</script>
    106107</form>
    107108</body>
    108 </html>
    109  No newline at end of file
     109</html>

Attachments (0)

Change History (7)

comment:1 Changed 14 years ago by CuriousCurmudgeon

Status: newassigned

comment:2 Changed 14 years ago by CuriousCurmudgeon

What was the bug you fixed with keyword modifications for multiple tickets? I see the changes you made, but I don't know what problems you were seeing.

The rest of it looks good. You beat me to implementing the configuration options. I was going to do that myself soon, but you saved me the work. :)

comment:3 Changed 14 years ago by CuriousCurmudgeon

I am unable to apply the patch for some reason. Can you try doing an update and attaching the .patch file to the ticket?

comment:4 Changed 14 years ago by CuriousCurmudgeon

(In [8120]) refs #7181

  • Applied patch to 0.11 and 0.12 versions. It wouldn't apply automatically for some reason so I did it manually.

comment:5 in reply to:  2 ; Changed 14 years ago by anonymous

Replying to CuriousCurmudgeon:

What was the bug you fixed with keyword modifications for multiple tickets? I see the changes you made, but I don't know what problems you were seeing.

I guess you've already noticed... You use 'merge_keywords and populate' like:

values['keywords'] = merge_keyowrds(t.values['keywords'], values['keywords'])
t.populate(values)

But 'values' was never initialized for each tickets, so the 'merged' result was used for 'requests' for the following tickets.

For example, you have two tickets with keywords

  • #1 - a,b
  • #2 - c,d

and you 'batchmod' w/ 'e,-b'. After the plugin processes #1, 'values' became 'a,e' as the result of 'merge_keyword'.
And it was applied to #2 as the 'request', so #2 came 'c,d,a,e'.

So I changed to preserve the original requests.

One more changes is to keep the original order as possible. You used 'set()' and it's completely correct for in this usage, but I want to keep original order, it's just my flavor but for non-alphabet keywords, simple 'sort()' doesn't work and I didn't have good way to give some order to keywords....

comment:6 in reply to:  5 Changed 14 years ago by beachroad

Replying to anonymous:

Replying to CuriousCurmudgeon:

Oops. I forgot to login before add a comment..

comment:7 Changed 14 years ago by CuriousCurmudgeon

Resolution: fixed
Status: assignedclosed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain CuriousCurmudgeon.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.