Re: [Apache Bloodhound] #710: Failed to batch modify tickets to set resolution=duplicate

[Ryan Ollos <ry...@wandisco.com>]

On Mon, Nov 11, 2013 at 12:41 AM, Ryan Ollos <ry...@wandisco.com>wrote:

> On Fri, Nov 8, 2013 at 7:17 AM, Gary Martin <ga...@wandisco.com>wrote:
>
>> On 07/11/13 20:56, Olemis Lang wrote:
>>
>>> On Tue, Nov 5, 2013 at 2:59 AM, Ryan Ollos <ry...@wandisco.com>
>>> wrote:
>>>
>>>  On Mon, Nov 4, 2013 at 11:57 PM, Apache Bloodhound <
>>>> dev@bloodhound.apache.org> wrote:
>>>>
>>>>  #710: Failed to batch modify tickets to set resolution=duplicate
>>>>> ---------------------------+----------------------------------
>>>>>    Reporter:  olemis        |      Owner:  olemis
>>>>>        Type:  defect        |     Status:  closed
>>>>>    Priority:  blocker       |  Milestone:  Release 8
>>>>>   Component:  multiproduct  |    Version:  0.7.0
>>>>> Resolution:  fixed         |   Keywords:  ticket, batch modify
>>>>> ---------------------------+----------------------------------
>>>>> Changes (by rjollos):
>>>>>
>>>>>   * owner:   => olemis
>>>>>
>>>>>
>>>>> --
>>>>> Ticket URL: <https://issues.apache.org/bloodhound/ticket/710#comment:6
>>>>> >
>>>>> Apache Bloodhound <https://issues.apache.org/bloodhound/>
>>>>> The Apache Bloodhound issue tracker
>>>>>
>>>>>
>>>> As to fixing the underlying problem, that the duplicate ticket relation
>>>> isn't added to the `ticket_change` table, I started working on fixing
>>>> that
>>>> and my changes so far can be found in (1). `ITicketManipulator` isn't
>>>> called on batch modification, which presents a bit of a problem since
>>>> the
>>>> `duplicate` attribute is added during the validation. There doesn't
>>>> seem to
>>>> be any way to validate batch modifications. I'm led to the conclusion
>>>> that
>>>> we need to either implement ITicketManipulator for batch modifications,
>>>> or
>>>> add a different extension point, `IBatchTicketManipulator`.
>>>>
>>>> (1) https://github.com/rjollos/bloodhound/compare/trunk...t710
>>>>
>>>
>>> The resource changed interfaces discussed in t.e.o issue tracker should
>>> deal with batch updates . I'm not fond of adding new interfaces to do
>>> exactly the same thing others do ... though I might be missing something
>>>
>>>
>> There is only one ticket with a mention of IResourceChanged and that is
>> pretty old. Is there something else I should be looking for? If I
>> understand the issue it looks like something that should be fixed fairly
>> soon though it doesn't seem to be a blocker.
>>
>> Cheers,
>>     Gary
>>
>
> The Trac ticket Olemis was referring to is #11148. I'm not sure the
> ResourceChangeListener will work in this case, since we need to intercept
> the event before the resource is changed.
>
> However, I think it will be possible to fix this without needing another
> hook since we already subclass BatchModifyModule in
> multiproduct.ticket.batch and can do the validation directly in the request
> handler.
>

The implementation isn't as clean as I had hoped, but we can override
`_save_ticket_changes` and directly call
`TicketRelationsSpecifics.validate_ticket` on each ticket. I'm targeting
this as a short-term solution, with something implemented in the Trac core
as the long term solution, either calling ITicketManipulators in the
BatchModifyModule, or using the IResourceChangeListeners.

Batch modification at global scope frequently results in "Error: Invalid
ticket number". I'm looking into that issue as well before posting my
proposed changes.