Project

General

Profile

Actions

Task #2072

open

Extend monitored fields with extended attribute processor

Added by Marek Klement about 4 years ago. Updated over 3 years ago.

Status:
New
Priority:
Normal
Assignee:
Radek Tomiška
Category:
Notification
Target version:
-
Start date:
02/19/2020
Due date:
% Done:

0%

Estimated time:
Owner:

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

Related to extras - Task #2071: Create processor to notify role holder when extended attribute changedRejectedMarek Klement02/19/2020

Actions
Related to IdStory Identity Manager - Task #2123: Save originalSource before change for IdmFormInstanceDtoNewRadek Tomiška03/17/2020

Actions
Actions #1

Updated by Marek Klement about 4 years ago

  • Related to Task #2071: Create processor to notify role holder when extended attribute changed added
Actions #2

Updated by Marek Klement about 4 years ago

  • Processor now get change in formValue and try to notify holder of role
  • Second processor was created to save originalSource
TODO:
  • Multi eav definitions
  • Edit send notification
  • Create in product
  • Tests
  • Documentation
Actions #4

Updated by Marek Klement about 4 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
Actions #5

Updated by Marek Klement about 4 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

Actions #6

Updated by Marek Klement about 4 years ago

- We agreed with Ondra to extend FormAttribute for - checkbox for notification

Actions #7

Updated by Marek Klement about 4 years ago

This won't be implemented with the checkbox.
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
Actions #8

Updated by Marek Klement about 4 years ago

  • Related to Task #2123: Save originalSource before change for IdmFormInstanceDto added
Actions #9

Updated by Marek Klement about 4 years ago

Actions #10

Updated by Marek Klement about 4 years ago

Notes:
  • 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
Actions #11

Updated by Marek Klement about 4 years ago

  • Status changed from In Progress to Needs feedback
  • Assignee changed from Marek Klement to Radek Tomiška
Actions #12

Updated by Radek Tomiška about 4 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.

Actions #13

Updated by Radek Tomiška over 3 years ago

  • Status changed from In Progress to New
  • Assignee changed from Marek Klement to Radek Tomiška
  • % Done changed from 30 to 0
Actions

Also available in: Atom PDF