Feature #2531
closed
New eIdentity driver for CRT module
100%
Description
- generate cupon,
- download lasted certificate,
- finish request for certificate with cupon and key from card - new card implementation.
Updated by Roman Kučera almost 4 years ago
- Assignee changed from Ondřej Kopr to Roman Kučera
Implementation in branch https://git.bcvsolutions.eu/modules/crt/-/commits/2531-eidentity-driver
Created driver with config. Did some basic PoC of the life cycle of crt request to know which methods in driver should be implemented.
Updated by Roman Kučera almost 4 years ago
- % Done changed from 10 to 80
Fixed bug, when you saw all requests on each dashboard.
The issue was in toFilter method in request controlled. Where the owner should be mapped from ownerId but was only owner. After changing everything is working correctly
Updated by Roman Kučera over 3 years ago
implementation is finished.
In this ticket dashboard for cards was also added for better UX.
Updated by Roman Kučera over 3 years ago
- Status changed from In Progress to Needs feedback
- Assignee changed from Roman Kučera to Ondřej Kopr
- % Done changed from 80 to 90
Updated by Ondřej Kopr over 3 years ago
- Status changed from Needs feedback to In Progress
- Assignee changed from Ondřej Kopr to Roman Kučera
It's little bit harder with testing this certification authority because eIdentity hasn't test environment. Made fakt driver is probably only chance how test at least something.
I tested whole behavior on our project and it's works. After review I just found these minor things:- codes; WEB_SERVICE_INITIALIZATION_ERROR and GENERATE_CUPON_ERROR are not localized,
- code GENERATE_CUPON_ERROR has same description as WEB_SERVICE_INITIALIZATION_ERROR,
- for externalId isn't used interface ExternalIdentifiable for dto and for entity,
- for EIdentityUtils missing interface. It's not required to made this, but clean code :D - also some nice class java doc missing.
Otherwise all behavior works as we expect.
For most of drivers methods missing implementation and behavior return null. This is probably only behavior that can be used. Maybe better will be throw some exception like unsupported exception or etc. But null will also works.
Second package branch for eIdentity generated by wsdl its OK for me. I didn't check the code generated by wsdl. It's very similar like old behavior.
Thanks for your implementation!
Updated by Roman Kučera over 3 years ago
Thx for review, I did some fixes, need to fix the other things to.
https://git.bcvsolutions.eu/modules/crt/-/commit/337b36f39cea6377c53c42b3d495c68e6ce604e6
https://git.bcvsolutions.eu/modules/crt/-/commit/f1a2317c4e72e6dab3abbe701cdd0b272110e243
Ondřej Kopr wrote:
It's little bit harder with testing this certification authority because eIdentity hasn't test environment. Made fakt driver is probably only chance how test at least something.
I tested whole behavior on our project and it's works. After review I just found these minor things:
- codes; WEB_SERVICE_INITIALIZATION_ERROR and GENERATE_CUPON_ERROR are not localized, - there is low chance this error will occure so we skip localazation for now
code GENERATE_CUPON_ERROR has same description as WEB_SERVICE_INITIALIZATION_ERROR,for externalId isn't used interface ExternalIdentifiable for dto and for entity,for EIdentityUtils missing interface. It's not required to made this, but clean code :D - also some nice class java doc missing.there was one static and one non static methods. I made both as non static.Otherwise all behavior works as we expect.
For most of drivers methods missing implementation and behavior return null. This is probably only behavior that can be used. Maybe better will be throw some exception like unsupported exception or etc. But null will also works.
Second package branch for eIdentity generated by wsdl its OK for me. I didn't check the code generated by wsdl. It's very similar like old behavior.
Thanks for your implementation!
Updated by Vít Švanda over 3 years ago
I did review.
There is my notes:
- I'm not sure what FakeEmployeeCertificateManager is.
- EIdentityUtils/implementation doesn't have a JavaDoc.
- In DefaultEIdentityUtils is comment with connection to project.
- I aggree with Ondra, unsupported methods should be return exception instead null (comments '// TODO Auto-generated method stub' are not nice too). Are you sure that other methods will not be needed sometime in the future? This looks like only one "project" implementation.
- Add field InternalId on the request detail is correct, but localization as 'Coupon number' isn't universal :-(.
Updated by Roman Kučera over 3 years ago
Vít Švanda wrote:
I did review.
There is my notes:
I'm not sure what FakeEmployeeCertificateManager is.Moved to projectEIdentityUtils/implementation doesn't have a JavaDoc.addedIn DefaultEIdentityUtils is comment with connection to project.removedI aggree with Ondra, unsupported methods should be return exception instead null (comments '// TODO Auto-generated method stub' are not nice too). Are you sure that other methods will not be needed sometime in the future? This looks like only one "project" implementation.added errors, but in method getCertificateForCA there have to be null otherwise request to eidentity will fail.Add field InternalId on the request detail is correct, but localization as 'Coupon number' isn't universal :(.- Renamed to External identifier
Thx for feedback. I improved the quality of code, so it should be closer to product standard :) some stuff was more of a project implementation so I moved it to project
Next I'll merge it to develop so I can check Sonar.
https://git.bcvsolutions.eu/modules/crt/-/commit/ec6d9d228159f33f0d19f5d81539cae457b2451e
Updated by Roman Kučera about 3 years ago
- Status changed from In Progress to Closed
- % Done changed from 90 to 100
already released, closing