IdStory IdM modules » extras

I did code review and here are some notes. Most of them is just code style related things and typos.

DefaultExtrasWfApprovingService:48 add private access modifier
DefaultExtrasWfApprovingService:61 typo in guarenteeType should be guaranteeType :)
DefaultExtrasWfApprovingService:71 use english names for variables
DefaultExtrasWfApprovingService:72 use english names for variables
DefaultExtrasWfApprovingService:78 typo in guarenteeType should be guaranteeType :)
DefaultExtrasWfApprovingService:95 typo in guarenteeType should be guaranteeType :)
DefaultExtrasWfApprovingService:103 typo in guarenteeType should be guaranteeType :)
DefaultExtrasWfApprovingService:185 you can use method reference instead of lambda, just depend on personal preferences.

some method name has spaces before ( some don't, just use some auto format, so the code will have the same style

This condition is little confusing to me. Why is fallback to returning guarantees without type inside condition for type A? In case that Type is B but B has no guarantees you fill empty list into result. And in case that type is B but you have no guarantees for A or B you have no fallback for guarantees without type.

        if (guarenteeType.equals(APPROVE_BY_GUARANTEE_A)) {    
            if (!guaranteesA.isEmpty()) {
                result = guaranteesA;
            //if we do not have A type or B type guarantees. We return all guarantees (no type specified) 
            } else if (guaranteesA.isEmpty() && guaranteesB.isEmpty()) {
                result.addAll(getRoleGuaranteesByType(role, null));
            }
        }    

        if (guarenteeType.equals(APPROVE_BY_GUARANTEE_B)) {
            result = guaranteesB;
        }

DefaultExtrasWfApprovingService::guaranteeCheck I don't see any advantage of recalling other public methods from this "central" method. It would be better to call the public methods directly from WF. To change this you will need to do some changes to service and to WF and retest it.

One test failed on my local machine approvalByRoleGuranteeWithouGuaranteesByType otherwise coverage is nice.
ExtrasWfApprovingServiceTest.java:138
org.junit.ComparisonFailure: expected:<guaranteeIdentity[]> but was:<guaranteeIdentity[TypeA]>
Expected :guaranteeIdentity
Actual :guaranteeIdentityTypeA

I fixed and re-tested all comments from review. Condition for managers by type was explained on call.
Commit: https://github.com/bcvsolutions/czechidm-extras/commit/45e70a3a9ef4889af87a523325261052fa9bddb2

Please @kucerar can you make a final review?

I checked the fixes and it looks good.
Even all tests are successful.
Thx, for this feature.