Events integration #64
Conversation
| */ | ||
| private void changeProductCommittee(Product product, ProductEventsSync productEventsSync) { | ||
| CommitteeEnum committeeEnum = CommitteeEnum.valueOf(productEventsSync.getOrganizedBy()); | ||
| int year = 2017; |
There was a problem hiding this comment.
While this implementation is very specific for CH Events, the code looks good and is simple enough to maintain. I'd prefer a more generic product creation API with managed tokens for access, but I also think this is good enough while CH Events is the only system actually using this. It's also easy enough to modify when another service is going to this later on.
Some small requested changes in the comments, and I'm missing a test for the edit functionality
| } | ||
|
|
||
| switch (productEventsSync.getTrigger()) { | ||
| case "PRODUCT_CREATE_EDIT": |
|
|
||
| switch (productEventsSync.getTrigger()) { | ||
| case "PRODUCT_CREATE_EDIT": | ||
| this.determineCreateOrUpdate(productEventsSync); |
There was a problem hiding this comment.
You're not determining anything, you're creating or updating it. createOrUpdate() works just fine here.
There was a problem hiding this comment.
Fix in commit b9e45db
|
|
||
| @PostMapping("/product/") | ||
| public ResponseEntity<?> eventSync(HttpServletRequest request, @RequestBody ProductEventsSync productEventsSync) { | ||
| String[] credentials = this.decryptBasicAuthHeader(request.getHeader("Authorization")); |
There was a problem hiding this comment.
This feels ugly, but it's quick and effective I guess... I'd prefer a more native Spring way using filters and an in-memory events user.
| return createResponseEntity(HttpStatus.BAD_REQUEST, "Events trigger not supported!", null); | ||
| } | ||
|
|
||
| return createResponseEntity(HttpStatus.OK, null, null); |
There was a problem hiding this comment.
This should be more specific, with HttpStatus.CREATED when a new product is created
There was a problem hiding this comment.
This should be a HttpStatus.OK, otherwise CH events thinks the sync failed.
|
The implementation should be specific for CH Events. Because it should be an endpoint which Events call when something has been changed. |
|
|
Fixes #63
Description
Integration between Events and Payments.
Todos
Steps to Test or Reproduce
Run the tests.