Page Comparison

...

• Reverse the call from Reference Data to Auth Service.  Having this implies that to use Reference Data, you need to call a specific endpoint on auth
• Remove auth's hardcoded list of services
• Auth endpoints regarding initiate user password, change password, and forgot password aren't very restful

Stockmanagement Service

• Remove auth's hardcoded list of services
• Auth endpoints regarding initiate user password, change password, and forgot password aren't very restful

Fulfillment Service

• Convert to order should be a fulfillment right. It is by warehouse, not by program. 
• Need an integration test for FacilityFtpSettingRepository.findFirstByFacilityId (done: test)
• Order domain object should not use Json annotations, but DTO for serial/deserial
• Remove all hardcoded English strings that get returned to the client 

  Jira Legacy
  server JIRA (openlmis.atlassian.net) columns key,summary,type,created,updated,due,assignee,reporter,priority,status,resolution serverId 448ba138-230b-3f91-a83e-16e7db1deed1 key OLMIS-1399

• Remove extra headers (X-Content-Type-Options, X-XSS-Protection) in RAML (done: review)
• All endpoints should have controller integration tests for different HTTP codes; add JSON schemas (generally a regular JSON for 2XX codes, and errorResponse for 4XX codes) 

  Jira Legacy
  server JIRA (openlmis.atlassian.net) columns key,summary,type,created,updated,due,assignee,reporter,priority,status,resolution serverId 448ba138-230b-3f91-a83e-16e7db1deed1 key OLMIS-1499

• Remove 500 errors from RAML (any endpoint can return it, but we do not have integration tests for these cases)
• Change errorResponse.json to have message and messageKey, not message and description 

  Jira Legacy
  server JIRA (openlmis.atlassian.net) columns key,summary,type,created,updated,due,assignee,reporter,priority,status,resolution serverId 448ba138-230b-3f91-a83e-16e7db1deed1 key OLMIS-1399

• Error handling classes should not log error, but should log debug (these are not actually errors)
• Order.status should not be settable from a DTO.
• Need to redesign to remove Dtos from Order domain object.
• Doesn't make sense for StatusMessage object to be part of Order object. Doesn't seem to be used anywhere and if it's needed, it should be retrieved from the Requisition service.
• Doesn't make sense for StatusChange object to be part of Order object. Only thing that appears to need it is the order report, and that could be filled in the DTO. It should be retrieved from the Requisition service.
• It's unclear what facilityId is used for and why it's necessary, since there is requestingFacilityId, supplyingFacilityId and receivingFacilityId.
• Move all authentication stuff in OrderController and ProofOfDeliveryController to PermissionService.
• OrderController.getAllOrders should be paginated, if we need it. We may not want to have it, and just have everyone use the order search endpoint.
• ExporterBuilder seems strange, as every time you need to convert something to DTO, you have to add another method to it.
• Don't quite understand the need for Storable, since TransferProperties is the only one that uses it. Is this abstraction necessary?
• Don't quite understand the need for OrderSender, since OrderFtpSender is the only one that uses it. Is this abstraction necessary?
• Don't quite understand the need for TransferPropertiesFactory and TransferPropertiesConverter. The code is hard to follow.
• Update README (stale)

...

• When configuring a requisition template, the endpoint needs to check for valid IDs of program and facility type
• Calculated fields are using Helper classes, not putting all business logic inside the Domain objects (let's ask for a meeting to explain it; Chongsun said the code is hard to follow)
• PermissionService.hasPermission by requisitionId should throw exception if requisition is null (otherwise hasPermission continues without problems even though ID is not valid) Moved into OLMIS-1859
• RequisitionService.convertToOrder should be releasing a requisition in the domain layer (requisition.release(), or requisitionReleaser.release(requisition)), not in the service layer (which just sets status)
• /requisitions/submitted should go away; it can be handled by the /requisitions/search endpoint with a filter
• BooleanResultDto should be refactored to ResultDto<Boolean>
• PermissionServiceTest.hasRight should have more descriptive name
• Remove all hardcoded English strings that get returned to the client
• Remove extra headers (X-Content-Type-Options, X-XSS-Protection) in RAML
• All endpoints should have controller integration tests for different HTTP codes; add JSON schemas (generally a regular JSON for 2XX codes, and errorResponse for 4XX codes)
• Remove 500 errors from RAML (any endpoint can return it, but we do not have integration tests for these cases)
• Change errorResponse.json to have message and messageKey, not message and description
• Error handling classes should not log error, but should log debug (these are not actually errors)
• Stock Adjustment Reasons are supposed to be configured by program but the API endpoints do not specify a program (see OLMIS-381 and compare to Swagger/RAML for /api/stockAdjustmentReasons)
• Update README (stale)
• Remove DTOs from Requisition and RequisitionLineItem (and potentially other) domain objects. Instead, should snapshot the data needed and store it in those domain objects.
• For good OO, Requisition objects should be created with a processing period required (which would also populate the field numberOfMonthsInPeriod)
• Some requisition endpoints, the logic is in the RequisitionController and for some, the logic is in the RequisitionService and it is not clear why one is chosen vs. the other (ex: skip and delete has logic in the service, while submit, authorize, approve is in the controller). This is a problem when trying to create a helper method; then this helper method needs to be implemented in both the controller and service (breaking DRY), but the helper method does not quite fit in being a fully public method
• Controller integration tests are not mocking anything below the controller (not on Spring Boot 1.4, and not using MockBean annotation) this is OLMIS-1142
• Need to create status message controller integration tests once existing integration tests are using a mocked pattern
• RequisitionLineItems can be directly modified, but should be encapsulated inside Requisition logic. There are a lot of public methods in the RequisitionLineItem class.
• Need to create jasper template controller integration tests once existing integration tests are using a mocked pattern
• Requistion template endpoints use RequisitionTemplate domain object, not DTO this is OLMIS-2112

Reference Data Service

• Lot Codes were implemented (OLMIS-2262) with a validator that requires global uniqueness. That logic is probably incorrect. 

  • In LotValidator.java verifyCode() method, it forces each lot to be globally unique. I believe correct behavior is that LotCodes are only unique within a given Trade Items. Definitely two different TradeItems might both have a Lot Code "100", for example. OR perhaps we should have no constraints around uniqueness at all.

• /users/userId/fulfillmentFacilities needs rightId parameter (similar to /users/userId/supervisedFacilities)
• SupplyLine search has search and searchByUUID, which seems like it should be consolidated to one search endpoint.
• Remove all hardcoded English strings that get returned to the client
• Remove extra headers (X-Content-Type-Options, X-XSS-Protection) in RAML
• All endpoints should have controller integration tests for different HTTP codes; add JSON schemas (generally a regular JSON for 2XX codes, and errorResponse for 4XX codes)
• Remove 500 errors from RAML (any endpoint can return it, but we do not have integration tests for these cases)
• Change errorResponse.json to have message and messageKey, not message and description
• Error handling classes should not log error, but should log debug (these are not actually errors)
• Update README (stale)

...