Opened 4 years ago

Closed 4 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


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/

    33# Copyright (C) 2007 Dave Gynn 
    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) 
     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") 
    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')) 
    4052        return handler 
    4254    def post_process_request(self, req, template, content_type): 
    7587                if not modify_changetime: 
    7688                  original_changetime = to_timestamp(t.time_changed) 
    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]) 
    81                 t.populate(values) 
     94                t.populate(_values) 
    8295                t.save_changes(req.authname, comment) 
    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) 
    110         regexp = re.compile(r'[^-\w]+') 
     123        regexp = re.compile(self.list_separator_regex) 
    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] 
    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) 
    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) 
    126140    # ITemplateStreamFilter methods 
    127141    def filter_stream(self, req, method, filename, stream, formdata): 
    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() 
    139153        ticketSystem = TicketSystem(self.env) 
    140154        fields = [] 
  • batchmod/templates/batchmod.html

    55     xmlns:py="" 
    66     xmlns:xi=""> 
    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')" 
    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 
    108 </html> 
    109  No newline at end of file 

Attachments (0)

Change History (7)

comment:1 Changed 4 years ago by CuriousCurmudgeon

  • Status changed from new to assigned

comment:2 follow-up: Changed 4 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 4 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 4 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 ; follow-up: Changed 4 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'])

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 4 years ago by beachroad

Replying to anonymous:

Replying to CuriousCurmudgeon:

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

comment:7 Changed 4 years ago by CuriousCurmudgeon

  • Resolution set to fixed
  • Status changed from assigned to closed

Add Comment

Modify Ticket

as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from CuriousCurmudgeon. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.

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

Note: See TracTickets for help on using tickets.