Project

General

Profile

Actions

Task #2274

closed

Refactor ImportRolesFromCSVExecutor and CSVToIdM

Added by Peter Štrunc over 4 years ago. Updated over 4 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Peter Štrunc
Target version:
Start date:
05/26/2020
Due date:
% Done:

100%

Estimated time:
Owner:

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

Related to extras - Task #2237: Extend ImportRolesFromCSVExecutor with guarante-typeClosedTomáš Doischer05/04/2020

Actions
Related to extras - Defect #2292: Role import with multivalue values separated with | creates multiple roles with one character namesClosedTomáš Doischer06/03/2020

Actions
Blocked by extras - Task #2270: Add option to ImportRolesFromCSVExecutor to specify column for MemberOf attribute valueClosedPeter Štrunc05/25/2020

Actions
Actions #1

Updated by Peter Štrunc over 4 years ago

  • Related to Task #2237: Extend ImportRolesFromCSVExecutor with guarante-type added
Actions #2

Updated by Tomáš Doischer over 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.

Actions #3

Updated by Tomáš Doischer over 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.

Actions #4

Updated by Vladimír Kotýnek over 4 years ago

  • Related to Defect #2292: Role import with multivalue values separated with | creates multiple roles with one character names added
Actions #5

Updated by Peter Štrunc over 4 years ago

  • Target version changed from 2.3.0 to 2.4.0
Actions #6

Updated by Tomáš Doischer over 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?

Actions #7

Updated by Peter Štrunc over 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.

Actions #8

Updated by Peter Štrunc over 4 years ago

  • Blocked by Task #2270: Add option to ImportRolesFromCSVExecutor to specify column for MemberOf attribute value added
Actions #9

Updated by Tomáš Doischer over 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. :)

Actions #10

Updated by Peter Štrunc over 4 years ago

  • Status changed from In Progress to Closed
Actions

Also available in: Atom PDF