Task #2274
closed
Refactor ImportRolesFromCSVExecutor and CSVToIdM
100%
Description
The goal to this ticket is to refactor ImportRolesFromCSVExecutor and CSVToIdM so that it is more readable and clean and to fix sonar issues which are currently in thoese two classes.
Related issues
Updated by Peter Štrunc almost 4 years ago
- Related to Task #2237: Extend ImportRolesFromCSVExecutor with guarante-type added
Updated by Tomáš Doischer almost 4 years ago
Improve the tutorial as well. There are attributes which only make sense when used together. The tutorial needs to do a better job of explaining each attribute.
Updated by Tomáš Doischer almost 4 years ago
@kotynekv noticed an issue with the separator for multivalued attributes, special characters (|) cause issues. They are not escaped in the code, I will fix this in this ticket.
Updated by Vladimír Kotýnek almost 4 years ago
- Related to Defect #2292: Role import with multivalue values separated with | creates multiple roles with one character names added
Updated by Peter Štrunc almost 4 years ago
- Target version changed from 2.3.0 to 2.4.0
Updated by Tomáš Doischer almost 4 years ago
- Status changed from New to Needs feedback
- Assignee changed from Tomáš Doischer to Peter Štrunc
- % Done changed from 0 to 80
I refactored the code, it can be found here: https://github.com/bcvsolutions/czechidm-extras/tree/2274-refactor-import-roles
The branch is based on the branch '2270-value-of-memberOf-import' and the task #2270.
Thing I've improved:- I rewrote the class CSVToIdM: It is now much easier to add a new column/attribute to the task.
- I refactored setGuarantees and setRoleGuarantees in ImportRolesFromCSVExecutor, got rid of the nested if/else blocks. As a result, some information is lost (it is impossible to say what failed without those blocks unless some other logic is implemented) but that shouldn't be an issue.
- I fixed Sonar bugs.
- I fixed #2292 caused by unescaped special characters.
- I finally added localization to the task.
- I improved the documentation.
@sourek, can you please do a code-review?
Updated by Peter Štrunc almost 4 years ago
- Status changed from Needs feedback to In Progress
- % Done changed from 80 to 100
Great job @doischert. It is much cleaner looking now and more readable too.
I have some trouble merging your branch though. It seems that it does not originate from develop branch, but rather from origin/2270-value-of-memberOf-import which contains some commits, which have not been reviewed yet. Lets wait for 2270 to be finished and then we can merge this feature to develop.
Updated by Peter Štrunc almost 4 years ago
- Blocked by Task #2270: Add option to ImportRolesFromCSVExecutor to specify column for MemberOf attribute value added
Updated by Tomáš Doischer almost 4 years ago
@sourek Thank you, can you also check #2270? I've already seen the code and it looks fine but you should confirm it. :)
Updated by Peter Štrunc almost 4 years ago
- Status changed from In Progress to Closed