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 .
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.