Task #2993
closed
LRT notification for new contracts/accounts
Added by Roman Kučera about 3 years ago.
Updated about 3 years ago.
Description
It's similar task to LastContractEndNotificationTask but only for creation
LRT params:
- Number of days before contract is valid 0=now, -X=afte valid contract X=before valid contract
- Recipient, by role(selectbox), manager(checkbox), directly to the user(checkbox)
- name of topic
- Select box with systems - if no system is selected all accounts will be shown
- checkbox if you want only contracts - no accounts, only contracts will be shown
- Type of user - form projection
params for notification same as in LastContractEndNotificationTask
- Description updated (diff)
- % Done changed from 0 to 60
- % Done changed from 60 to 80
- Status changed from In Progress to Needs feedback
- Assignee changed from Roman Kučera to Tomáš Doischer
- Status changed from Needs feedback to In Progress
- Assignee changed from Tomáš Doischer to Roman Kučera
Thank you for this feature. This review is also for #2612.
I really like the use of negative numbers in configuration!
- are you sure removing the contract from the queue is desirable in ContractStartNotificationTask? I know that validFrom of contract can potentially change but is it really necessary to send the notification again if that happens? It is needed in LastContractEndNotificationTask but it does have performance impact.
- in both tasks, I would expect that tree node selection is used recursively, i. e., I will see users with contract on selected position and on those below it
- unused imports in ContractStartNotificationTask and LastContractEndNotificationTask
- Sonar has some issues (using boolean instead of Boolean)
- null pointer on line ContractStartNotificationTask:290 - validFrom can be null when called from getContractItemFromQueue()
Otherwise it looks great, I tested a few basic UCs and it works well.
Tomáš Doischer wrote in #note-5:
- are you sure removing the contract from the queue is desirable in ContractStartNotificationTask? I know that validFrom of contract can potentially change but is it really necessary to send the notification again if that happens? It is needed in LastContractEndNotificationTask but it does have performance impact.
The removing was implemented for use case if user will start working in company (notification is send) then he leave the company and he starts working there again. So we want to send the notification again. But when I am thinking about it now, I am not sure if he will have still the same contract. Because if if get a new contract then notification will be send anyway.
- in both tasks, I would expect that tree node selection is used recursively, i. e., I will see users with contract on selected position and on those below it
Yeah I thought that to. I will confirm this behavior with customer, or maybe some additional switch would be nice to choose if you want recursion or no.
- unused imports in ContractStartNotificationTask and LastContractEndNotificationTask
- Sonar has some issues (using boolean instead of Boolean)
- null pointer on line ContractStartNotificationTask:290 - validFrom can be null when called from getContractItemFromQueue()
I'll fix it thx.
Unused imports, sonar and null pointer is already fixed.
Fixes look great, thanks.
The other points I will leave up to you to decide whether to implement them or not.
LGTM, thank you for the fixes!
- Status changed from In Progress to Closed
- Target version set to 3.3.0
- % Done changed from 80 to 100
Also available in: Atom
PDF
