Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OH2-257 | Revise HTTP response codes in API #433

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 24 additions & 24 deletions src/main/java/org/isf/accounting/rest/BillController.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* Open Hospital (www.open-hospital.org)
* Copyright © 2006-2023 Informatici Senza Frontiere ([email protected])
* Copyright © 2006-2024 Informatici Senza Frontiere ([email protected])
*
* Open Hospital is a free and open source software for healthcare data management.
*
Expand Down Expand Up @@ -105,10 +105,10 @@ public BillController(BillBrowserManager billManager, PriceListManager priceList
* @throws OHServiceException
*/
@PostMapping(value = "/bills", produces = MediaType.APPLICATION_JSON_VALUE)
public ResponseEntity<FullBillDTO> newBill(@RequestBody FullBillDTO newBillDto) throws OHServiceException {
public ResponseEntity<?> newBill(@RequestBody FullBillDTO newBillDto) throws OHServiceException {

if (newBillDto == null) {
throw new OHAPIException(new OHExceptionMessage("Bill is null."));
return ResponseEntity.badRequest().body(new OHExceptionMessage("Bill is null."));
}
LOGGER.info("Create Bill {}", newBillDto);

Expand All @@ -123,13 +123,13 @@ public ResponseEntity<FullBillDTO> newBill(@RequestBody FullBillDTO newBillDto)
if (pat != null) {
bill.setBillPatient(pat);
} else {
throw new OHAPIException(new OHExceptionMessage("Patient not found."));
return ResponseEntity.badRequest().body(new OHExceptionMessage("Patient not found."));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find three different return values for something not found:

return ResponseEntity.badRequest().body("XXXX not found.");
return ResponseEntity.badRequest().body(new OHExceptionMessage("XXXX not found."));
return ResponseEntity.notFound().build();

Shouldn't there only be one form?

}

if (plist != null) {
bill.setPriceList(plist);
} else {
throw new OHAPIException(new OHExceptionMessage("Price list not found."));
return ResponseEntity.badRequest().body(new OHExceptionMessage("Price list not found."));
}

List<BillItems> billItems = billItemsMapper.map2ModelList(newBillDto.getBillItems());
Expand All @@ -139,7 +139,7 @@ public ResponseEntity<FullBillDTO> newBill(@RequestBody FullBillDTO newBillDto)
try {
billManager.newBill(bill, billItems, billPayments);
} catch (OHServiceException e) {
throw new OHAPIException(new OHExceptionMessage("Bill is not created."));
return ResponseEntity.internalServerError().body(new OHExceptionMessage("Bill is not created."));
}
return ResponseEntity.status(HttpStatus.CREATED).body(newBillDto);
}
Expand All @@ -152,15 +152,15 @@ public ResponseEntity<FullBillDTO> newBill(@RequestBody FullBillDTO newBillDto)
* @throws OHServiceException
*/
@PutMapping(value = "/bills/{id}", produces = MediaType.APPLICATION_JSON_VALUE)
public ResponseEntity<FullBillDTO> updateBill(@PathVariable Integer id, @RequestBody FullBillDTO odBillDto) throws OHServiceException {
public ResponseEntity<?> updateBill(@PathVariable Integer id, @RequestBody FullBillDTO odBillDto) throws OHServiceException {

LOGGER.info("updated Bill {}", odBillDto);
Bill bill = billMapper.map2Model(odBillDto.getBill());

bill.setId(id);

if (billManager.getBill(id) == null) {
throw new OHAPIException(new OHExceptionMessage("Bill to update not found."));
return ResponseEntity.badRequest().body(new OHExceptionMessage("Bill to update not found."));
}

Patient pat = patientManager.getPatientById(bill.getBillPatient().getCode());
Expand All @@ -172,13 +172,13 @@ public ResponseEntity<FullBillDTO> updateBill(@PathVariable Integer id, @Request
if (pat != null) {
bill.setBillPatient(pat);
} else {
throw new OHAPIException(new OHExceptionMessage("Patient not found."));
return ResponseEntity.badRequest().body(new OHExceptionMessage("Patient not found."));
}

if (plist != null) {
bill.setPriceList(plist);
} else {
throw new OHAPIException(new OHExceptionMessage("Price list not found."));
return ResponseEntity.badRequest().body(new OHExceptionMessage("Price list not found."));
}

List<BillItems> billItems = billItemsMapper.map2ModelList(odBillDto.getBillItems());
Expand All @@ -188,9 +188,9 @@ public ResponseEntity<FullBillDTO> updateBill(@PathVariable Integer id, @Request
try {
billManager.updateBill(bill, billItems, billPayments);
} catch (OHServiceException e) {
throw new OHAPIException(new OHExceptionMessage("Bill is not updated."));
return ResponseEntity.internalServerError().body(new OHExceptionMessage("Bill is not updated."));
}
return ResponseEntity.status(HttpStatus.CREATED).body(odBillDto);
return ResponseEntity.ok().body(odBillDto);
}

/**
Expand Down Expand Up @@ -222,7 +222,7 @@ public ResponseEntity<List<BillDTO>> searchBills(
List<BillDTO> billDTOS = billMapper.map2DTOList(bills);

if (billDTOS.isEmpty()) {
return ResponseEntity.status(HttpStatus.NO_CONTENT).body(billDTOS);
return ResponseEntity.notFound().build();
}
return ResponseEntity.ok(billDTOS);
}
Expand Down Expand Up @@ -255,7 +255,7 @@ public ResponseEntity<List<BillPaymentsDTO>> searchBillsPayments(
List<BillPaymentsDTO> paymentsDTOS = billPaymentsMapper.map2DTOList(payments);

if (paymentsDTOS.isEmpty()) {
return ResponseEntity.status(HttpStatus.NO_CONTENT).body(null);
return ResponseEntity.notFound().build();
}
return ResponseEntity.ok(paymentsDTOS);
}
Expand All @@ -276,7 +276,7 @@ public ResponseEntity<List<BillPaymentsDTO>> getPaymentsByBillId(@PathVariable(v
List<BillPaymentsDTO> paymentsDTOS = billPaymentsMapper.map2DTOList(billPayments);

if (paymentsDTOS.isEmpty()) {
return ResponseEntity.status(HttpStatus.NO_CONTENT).body(null);
return ResponseEntity.notFound().build();
}
return ResponseEntity.ok(paymentsDTOS);
}
Expand All @@ -296,7 +296,7 @@ public ResponseEntity<List<BillItemsDTO>> getItems(@PathVariable(value = "bill_i
List<BillItemsDTO> itemsDTOS = billItemsMapper.map2DTOList(items);

if (itemsDTOS.isEmpty()) {
return ResponseEntity.status(HttpStatus.NO_CONTENT).body(null);
return ResponseEntity.notFound().build();
}
return ResponseEntity.ok(itemsDTOS);
}
Expand All @@ -316,7 +316,7 @@ public ResponseEntity<BillDTO> getBill(@PathVariable Integer id) throws OHServic
BillDTO billDTO = billMapper.map2DTO(bill);

if (billDTO == null) {
return ResponseEntity.status(HttpStatus.NO_CONTENT).body(null);
return ResponseEntity.notFound().build();
}
return ResponseEntity.ok(billDTO);
}
Expand All @@ -336,7 +336,7 @@ public ResponseEntity<List<BillDTO>> getPendingBillsAffiliate(@RequestParam(valu
List<BillDTO> billDTOS = billMapper.map2DTOList(bills);

if (billDTOS.isEmpty()) {
return ResponseEntity.status(HttpStatus.NO_CONTENT).body(null);
return ResponseEntity.notFound().build();
}
return ResponseEntity.ok(billDTOS);
}
Expand All @@ -356,7 +356,7 @@ public ResponseEntity<List<BillDTO>> getPendingBills(@RequestParam(value = "pati
List<BillDTO> billDTOS = billMapper.map2DTOList(bills);

if (billDTOS.isEmpty()) {
return ResponseEntity.status(HttpStatus.NO_CONTENT).body(billDTOS);
return ResponseEntity.notFound().build();
}
return ResponseEntity.ok(billDTOS);
}
Expand Down Expand Up @@ -384,7 +384,7 @@ public ResponseEntity<List<BillDTO>> searchBills(
List<BillDTO> billDTOS = billMapper.map2DTOList(bills);

if (billDTOS.isEmpty()) {
return ResponseEntity.status(HttpStatus.NO_CONTENT).body(null);
return ResponseEntity.notFound().build();
}
return ResponseEntity.ok(billDTOS);
}
Expand All @@ -405,17 +405,17 @@ public ResponseEntity<List<BillItemsDTO>> getDistinctItems() throws OHServiceExc
List<BillItemsDTO> itemsDTOS = billItemsMapper.map2DTOList(items);

if (itemsDTOS.isEmpty()) {
return ResponseEntity.status(HttpStatus.NO_CONTENT).body(null);
return ResponseEntity.notFound().build();
}
return ResponseEntity.ok(itemsDTOS);
}

@DeleteMapping(value = "/bills/{id}", produces = MediaType.APPLICATION_JSON_VALUE)
public ResponseEntity<Boolean> deleteBill(@PathVariable Integer id) throws OHServiceException {
public ResponseEntity<?> deleteBill(@PathVariable Integer id) throws OHServiceException {
LOGGER.info("Delete bill id: {}", id);
Bill bill = billManager.getBill(id);
if (bill == null) {
return ResponseEntity.status(HttpStatus.NOT_FOUND).body(null);
return ResponseEntity.badRequest().body("No bill found with the specified id.");
}
try {
billManager.deleteBill(bill);
Expand All @@ -441,7 +441,7 @@ public ResponseEntity<List<BillDTO>> searchBillsByPayments(@RequestBody List<Bil
List<BillDTO> billDTOS = billMapper.map2DTOList(bills);

if (billDTOS.isEmpty()) {
return ResponseEntity.status(HttpStatus.NO_CONTENT).body(null);
return ResponseEntity.notFound().build();
}
return ResponseEntity.ok(billDTOS);
}
Expand Down
Loading
Loading