Task #11552
[SC REVIEW] SPRINT 1
| Status: | Code Review | Start date: | February 23, 2021 | |
|---|---|---|---|---|
| Priority: | Normal | Due date: | February 26, 2021 | |
| Assignee: | % Done: | 100% | ||
| Category: | - | Spent time: | 4.00 hours | |
| Target version: | - |
Description
Dear Chee Ping,
As BPS Sprint 1 has been completed, please help to perform source code review on the following:
- Download Bulk Payment Template
- Bulk Payment Template Prompt at Landing Page and Service Info
- Batch job to send new registered Corporate Code to BPS at EOD via file
- Maintenance for Corporate Code (Create and Edit)
Thank you.
History
#1
Updated by Rayvandy Gabbytian over 3 years ago
- Due date changed from February 23, 2021 to February 26, 2021
- Assignee set to Ngoh Chee Ping
#2
Updated by Rayvandy Gabbytian over 3 years ago
Adding more information:
Mercurial path and branch:
https://cloud.penril.my/hg/agrobank/agro_admin/ BPS
https://cloud.penril.my/hg/agrobank/agro-bps-cron/ default
https://cloud.penril.my/hg/agrobank/agro-bib BPS
https://cloud.penril.my/hg/agrobank/agro-esb BPS
#3
Updated by Ngoh Chee Ping over 3 years ago
- Status changed from New - Begin Life Cycle to Code Review
- % Done changed from 0 to 50
Finished review below items with output below.
https://cloud.penril.my/hg/agrobank/agro_admin/ BPS
https://cloud.penril.my/hg/agrobank/agro-esb BPS
IBAM Code Review Output++
ibssCompanyEnquiryEdit.jsp
1) Extra <tr> in line 569
IBSSAuthorizationDetailsRegisterCompanyServices.java
1) Line 374-376 Should use Map<String, String> instead of Map for those get from business options.
IBSSAuthorizationDetailsEditCompanyServices.java
1) Line 386-388 Should use Map<String, String> instead of Map for those get from business options.
IBSSCompanyEnquiryDetailServices.java
1) Line 390-392 Should use Map<String, String> instead of Map for those get from business options.
IBSSCompanyEnquiryEditConfirmService.java
1) Line 29-34 Should use Map<String, String> instead of Map for those get from business options.
IBSSCreateCompanyConfirmService.java
1) Line 42-44 and 48-50 Should use Map<String, String> instead of Map for those get from business options.
IBSSAuthorizationResultRegisterCompanyServices.java
1) Line 438 only check for not null, should check if response bean is null log error and display unsuccessful like line 444. If call to BPS timeout, response bean will be null and the system will ignore the error.
IBSSCompanyEnquiryEditResultServices.java
1) Line 754 only check for not null, should check if response bean is null log error and display unsuccessful like line 761. If call to BPS timeout, response bean will be null and the system will ignore the error.
IBSSRegisterCompanyResultServices.java
1) Line 601 only check for not null, should check if response bean is null log error and display unsuccessful like line 606. If call to BPS timeout, response bean will be null and the system will ignore the error.
2) Line 606 should not throw business exception, and just allow go to result page with message unsuccessful sent to BPS like edit function? If throw business exception there, user can click confirm button again in confirmation page.
#4
Updated by Ngoh Chee Ping over 3 years ago
- Assignee changed from Ngoh Chee Ping to Rayvandy Gabbytian
- % Done changed from 50 to 100
Finished review below items with output below. Please assign to a person to make the changes.
https://cloud.penril.my/hg/agrobank/agro-bps-cron/ default
https://cloud.penril.my/hg/agrobank/agro-bib BPS
BIB Code Review Output++
IBStatutoryDownloadAction.java
1) Line 54 no use. Just remove it.
BPS Cron Code Review Output++
pom.xml
1) No need to add agro-core dependency separately, because agro-esb already have dependency with agro-core. Just need to add dependency of agro-esb is enough
BPSMigrationService.java
1) Line 79 should not comment the response bean null checking.
2) Line 89 need to add on the company ID to know which company ID is getting timeout.
hibernate.cfg.xml
1) No need to have all the mapping, because the cron job only get company profile. Only add the required tables.