Open
Conversation
…ng rows - introduce business-key based lookup for own productions, deliveries and stocks - change delivery business key to use departure and arrival timestamps - implement create-or-update (upsert) logic in ExcelService for all imports - add tests verifying update paths for demand, production, delivery and stock
…ng rows - introduce business-key based lookup for own productions, deliveries and stocks - change delivery business key to use departure and arrival timestamps - implement create-or-update (upsert) logic in ExcelService for all imports - add tests verifying update paths for demand, production, delivery and stock
remove IAV config
Author
|
Hi, I run deploy.sh -ci the result you can find here: Best Greetings, |
Contributor
tom-rm-meyer-ISST
left a comment
There was a problem hiding this comment.
Thanks for the contribution. I think we need some further alignment on the topic, as this pr mixes two things:
- it fixes the bug that existing information (duplicates) terminate the import process non-gracefully.
- it introduces a feature that not all old information is removed, only duplicates are updated or created
The second point raises the following issues:
- The introduced business keys are inconsistent with what is changeable in the frontend (e.g. demand allows to change day, category, quantity in frontend vs. quantity, unit, supplier location, demand location in backend)
- The introduced feature changes the import dialog behavior (information is added and changed vs information is removed and added). There is no way of batch removal OR a way to see old data in the application.
- The introduced feature creates duplicates (e.g. updating the departure to an actual departure will result in a new departure instead of an update).
From my perspective we should go as follows:
- Fix the bug for now with only deleting all information first and then update it. Make the method transactional.
- Align on a concept how the import should work (e.g. together with the customer) and consider cross-effects (e.g. right now the change would lead to providing the patner also production information from last year if applied for one year).
I've sent out a link via mail to book an appointment for discussion of the feature.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixing the Issue #1119
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.