WIP: slapos_crm: Merge too similar alarms
slapos_crm_check_suspended_support_request_to_reopen is not required since the Event_checkStoppedToDeliver already trigger over events once they are stopped.
Update Event_checkStoppedToDeliver to re-open suspended tickets like it does for invalidated ones.
Fix up Event_checkStoppedToDeliver to deliver also Events from Suspended tickets (after re-open them).
I intend to use this on Regularisation Request also
/cc @romain
If I did 2 alarms, it was on purpose.
I'm sorry, Romain don't do mistakes, I forgot :) (joking)
I didn't realise that an event could be created directly on delivered state, so I won't merge the alarms.
Furthermore, I will just extend the existing ones to support Regularisation Requests so, since they also need to behave the same as Support Request.
added 4 commits
-
5584f7b0...ff8cd707 - 3 commits from branch
nexedi:master
- 69fd89f7 - slapos_crm: Merge too similar alarms
-
5584f7b0...ff8cd707 - 3 commits from branch
- Last updated by Rafael Monnerat
23 event.deliver(comment='Ticket was validated') 24 event.reindexObject(activate_kw=activate_kw) 25 return 26 27 else: 19 return 28 20 29 if support_request.getSimulationState() == 'validated': 30 # Event is more recent than the ticket 31 # Touch the ticket, to allow manager to see it as recent 32 # and deliver the event 33 support_request.edit(activate_kw=activate_kw) 34 event.deliver(comment='Ticket has been modified') 35 event.reindexObject(activate_kw=activate_kw) 36 return 21 if support_request.getSimulationState() in 'validated': @rafael sadly, this make me not trust this MR
It was a typo, nothing more them that.
Edited by Rafael Monnerat
- Last updated by Rafael Monnerat
1174 1174 self.assertEqual(event.getSimulationState(), "stopped") 1175 1175 self.assertEqual(support_request.getSimulationState(), "suspended") 1176 1176 event.Event_checkStoppedToDeliver() 1177 self.assertEqual(event.getSimulationState(), "stopped") 1178 self.assertEqual(support_request.getSimulationState(), "suspended") 1177 self.assertEqual(event.getSimulationState(), "delivered") @rafael you must explain to me why you change the behaviour I setup
We briefly talked, and I will revert the back the behaviour, probably.
In short, considering that the event is always stopped when created (which is not the case), this change would deliver the event created once a Support Request is suspended, not requiring the other alarm.
Since the event is directly delivered most of the time, it is better keep like you it is currently implemented.
[EDITED] It was not nicely written, and I understood things better now.
FYI: I also noticed that add a ticket already try to validate the ticket:
So:
- Can you confirm that
Event_checkStoppedToDeliver
is only targeting old events? Or Events not created via Panel (if any)? - Is there a need to keep validate call on Ticket_addSlapOSEvent, since we add the alarm?
- Should we allow the user re-open a closed ticket by posting a message? Currently, it is not possible, can you confirm it is intentional?
In any case, I am closing this, and continue to work on Regularisation Request specifics on !702 (merged)
Edited by Rafael Monnerat- Can you confirm that
Should we allow the user re-open a closed ticket by posting a message? Currently, it is not possible, can you confirm it is intentional?
It would be nice. But we can not change the alarm to also check
invalidated
Support Request
, as it would be too slow.As we do not have this use case yet, I prefer to not spend time on this.
Is there a need to keep validate call on Ticket_addSlapOSEvent, since we add the alarm?
The alarm was created because project's customers are not allowed to change the
Support Request
state.But you're right, not because of the tiny code duplication, but maybe because using
Ticket_addSlapOSEvent
onsuspended
Regularisation Request
will reopen it. Meaning, all automated alarms will be processed. I'm not sure at all this is the behaviour wanted.We can try to drop this
validate
call.Can you confirm that
Event_checkStoppedToDeliver
is only targeting old events? Or Events not created via Panel (if any)?It only targets
stopped
state, in order to try removing them from the worklist when not needed anymore.There is no idea of created via panel or not. And it does not only target old event (but I'm not sure what old means in the question).