Task #2072
open
Extend monitored fields with extended attribute processor
Added by Marek Klement almost 5 years ago. Updated about 4 years ago.
0%
Description
The processor will react when the eav is changed. After its change, it will send a notification to the holder of a particular role. This role, as well as the code of the eav and type of entity, will be provided by the configuration property.
Functionality such as IdentityMonitoredFieldsProcessor.
Related issues
Updated by Marek Klement almost 5 years ago
- Related to Task #2071: Create processor to notify role holder when extended attribute changed added
Updated by Marek Klement almost 5 years ago
- Processor now get change in formValue and try to notify holder of role
- Second processor was created to save originalSource
- Multi eav definitions
- Edit send notification
- Create in product
- Tests
- Documentation
Updated by Marek Klement almost 5 years ago
- Subject changed from Extend IdentityMonitoredFieldsProcessor with extended attribute to Extend monitored fields with extended attribute processor
- Description updated (diff)
- Status changed from New to In Progress
Updated by Marek Klement almost 5 years ago
Suggestions for improvement and discussion:
- The processor now can exist just for one entity at the same time - improve this to more
- Function to given code should be thrown away and replaces - not sure with what now
Updated by Marek Klement almost 5 years ago
- We agreed with Ondra to extend FormAttribute for - checkbox for notification
Updated by Marek Klement almost 5 years ago
Solution:
- The processor will contain also parser that resolves configuration property by its owner's type.
- Also instead of the first processor original source will be saved naturaly - ticket 2123
Updated by Marek Klement almost 5 years ago
- Related to Task #2123: Save originalSource before change for IdmFormInstanceDto added
Updated by Marek Klement almost 5 years ago
- Created resolution of configuration by its type - type is part of the path in the configuration
- Changed topic also for the type
- Adjusted tests to work properly
- Documented https://wiki.czechidm.com/tutorial/adm/set_notification_for_eav
Updated by Marek Klement almost 5 years ago
- it is located at branch klement/2072-extend-monitored-fields-for-eav
- There is also the second processor to save original source - have to be removed after
Updated by Marek Klement almost 5 years ago
- Status changed from In Progress to Needs feedback
- Assignee changed from Marek Klement to Radek Tomiška
Updated by Radek Tomiška almost 5 years ago
- Status changed from Needs feedback to In Progress
- Assignee changed from Radek Tomiška to Marek Klement
- Target version deleted (
10.2.0) - % Done changed from 0 to 30
I did test and code review. This feature basically works, when extended attributes are saved from frontend, nice!
But The main issue is it doesn't work from every places in application (from synchronization by example). The issue is on the start in design of this feature (sorry that I hadn't seen it before).
Is not possible to implement this feature fully until at least synchronization will be redesigned in product to save whole FormInstance together (or better save whole entity with eavs are included). Now is saved attribute one by one without event usage => is no possible to listen some event and is not posible to load original value too (listening other event don't help here).
Maybe the whole concept of saving standalone attribute should be prohibited ... but this changes everything.
Other review notes:
Major:
- Overriden FormInstanceMonitoredFieldsProcessor#getName() method is missing => then configuration service is used wrongly. All configuration properties should have processors name space. See other processors for inspiration.
- Overriden FormInstanceSaveOriginalSourceProcessor#getName() method is missing, but FormInstanceSaveOriginalSourceProcessor is nicely implemented otherwise.
- FormInstanceMonitoredFieldsProcessor#process should return, if no recipient is found (maybe WARN level can be used, bacause values to monitor is configured, but recipients not).
- Default notification template is missing in product (topic initialization missing too, but is dynamic). Feature in product could work by default (available, but not not active until some configuration is added)
Minor:
- FormInstanceMonitoredFieldsProcessor#process row #94 - String.format() can be used for concat strings.
- Javadoc is missing (e.g. EavValue).
- getConfigurationService().getValues(...) never return null (CollectionUtils can be used otherwise on other places, where null can appear).
- for find identities - IdmIdentityFilter#setRoles can be used instead iteration through all roles one by one. Max limit could be set to (e.g. 50) to prevent send a lot of notifications.
- Some constant (e.g. minus 1500) is better than Integer.MIN_VALUE usage as processor order. Order is shown on FE and will be editable in future by.
- FormInstanceMonitoredFieldsProcessor#process - checking configuration (processor has to be executed) can be moved to #conditional method.
Now is needed to decide, what we need to cover and when. Then we can continue with chosen direction.
Updated by Radek Tomiška about 4 years ago
- Status changed from In Progress to New
- Assignee changed from Marek Klement to Radek Tomiška
- % Done changed from 30 to 0