feat(batch): prepare backend#1105
feat(batch): prepare backend#1105tom-rm-meyer-ISST merged 4 commits intofeat/1103-partner-update-batchfrom
Conversation
|
@ReneSchroederLJ The tests fail due to not providing an implementation for the ...BatchProcessService that is handed with #1106 |
ReneSchroederLJ
left a comment
There was a problem hiding this comment.
Overall great start for the feature. I've got some minor feedback to consider.
|
|
||
| @NotNull | ||
| @Column(nullable = false) | ||
| private UUID partnerId; |
There was a problem hiding this comment.
You're adding a lot of the partner's properties directly to this entity instead of creating a ManyToOne relationship. do you have performance concerns in this regard? Also I would argue that partnerId, while internally used as key, is not really necessary information in this object.
There was a problem hiding this comment.
Reasons I considered:
- performance (in Testimonial we already have 25 materials with each 4 information on at least a daily run) and loser coupling
- relationship is commonly not used as this is a write once entity (like audit information) -> no real benefit in relation
- eases handling a little
But you're right, if needed we can lookup with partnerBpnl. Removed that also from changelog.
| /** | ||
| * Direction used across domains. INBOUND = partnerSuppliesFlag, OUTBOUND = partnerBuysFlag | ||
| */ | ||
| public enum DirectionEnum { |
There was a problem hiding this comment.
This enum is an exact duplication of the DirectionCharacteristic from ItemStockSamm, which has been used in various places before. It could make sense to move the existing one to common like we've done for ItemUnitEnumeration and ItemQuantityEntity. Arguably this should have happened before anyway.
There was a problem hiding this comment.
Short discussion: Yes we should have it in common and used on other entities and dtos. My only concern is about the samm. Currently we used the autogeneration feature to some extent. I would not expect it to change really but as this is a definition that's only partially in our responsibility I would go for one DirectionCharacteristic for the samm and one DirectionEnum. Do you agree or shall I not overengineer and just go for it?
There was a problem hiding this comment.
raised #1109 for refactoring. Not handled with this pr
| "/parttypeinformation/**", | ||
| "/files/**" | ||
| "/files/**", | ||
| "/admin/**" |
There was a problem hiding this comment.
Does it really make sense to use the /admin route for this feature specifically? Other previously available features are also fully admin only.
There was a problem hiding this comment.
Switched to batch
66236df to
40a36b9
Compare
40a36b9 to
4042d5f
Compare
ReneSchroederLJ
left a comment
There was a problem hiding this comment.
LGTM. good baseline for the feature
c2fc6d0
into
feat/1103-partner-update-batch
* feat(batch): prepare backend (#1105) * feat(batch): prepare backend * chore: update changelog * fix: incorporate review * chore: add license header * feat(batch): implement batch processing, controllers and tests (#1106) * feat(batch): prepare backend * chore: update changelog * fix: incorporate review * chore: add license header * feat(batch): add controllers and tests * chore: update changelog * fix(PartnerDataUpdateBatchProcessServiceTest): switch mixed direction for DoS * fix: incorporate changes from othter pr * chore: cleanup and review incorp * feat(batch): implement frontend, cleanup scheduler and documentation (#1107) * feat(batch): prepare backend * chore: update changelog * fix: incorporate review * chore: add license header * feat(batch): add controllers and tests * chore: update changelog * fix(PartnerDataUpdateBatchProcessServiceTest): switch mixed direction for DoS * fix: incorporate changes from othter pr * feat(batch): add view to frontend * fix: incorporate changes from batch * feat: added retention scheduler * docs(batch): document changes * feat(batch): add sorting for batch runs * chore: cleanup and review incorp * refactor(BatchView): improve type safety Co-authored-by: René Schröder <131770181+ReneSchroederLJ@users.noreply.github.com> * fix: improve typization and fix pagination * fix(PartnerDataUpdateBatchProcessServiceImpl): prevent parallel runs * chore: bump vulnerabilities in frontend --------- Co-authored-by: René Schröder <131770181+ReneSchroederLJ@users.noreply.github.com> * docs(Migration_Guide.md): added duplicate exception note for operational data post endoints * fix(values.yaml): removed linting issue trailing spaces --------- Co-authored-by: René Schröder <131770181+ReneSchroederLJ@users.noreply.github.com>
Description
Warning
This is not compilable. Due to size of pr I left out processing service implementation.
solves #1104
Provides:
PartnerDataUpdateBatchRunwithPartnerDataUpdateBatchRunand respective datatypesPartnerDataUpdateBatchRunwithPartnerDataUpdateBatchRunDtobatch.partnerdataupdate.enabled,cronandretentiondaysapplication.propertiesfix for missing attribute in
MaterialPartnerRelation.toString()Pre-review checks
Please ensure to do as many of the following checks as possible, before asking for committer review:
changelog.md) with PR reference and brief summary.frontend/package.json,frontend/package-lock.json)backend/pom.xml)scripts/generate_openapi_yaml.pywith running customer backend)