Saving datamodel along with list of changed fields and previous values #899
Conversation
…ports (this will let us feature-toggle and deliver functionality that depend on future backend features).
…art request, making it possible to test this functionality before backend support arrives.
…effect on anything. Removing it.
…to save data before the previous one has ended. This could cause corruption or loss of data, and we can't guarantee the server-side responds to it correctly either. Using takeEvery instead of takeLatest, as takeLatest will abort a saga (effectively ignoring the 303 response back from the server).
…a state and update it when there are changes made to it
…y. It now makes sue to check that every request is fired, and that adding rows with data (and our nested group with preSelectedOptionsId) is saved correctly. This uncovered a lot of edge-cases!
… from resetting the state
…it tests that submitting form data behaves differently in the cases of 3 different app types (stateful, stateless and anonymous stateless). This also happens to match exactly with our 3 major cypress test apps, so we know we have code coverage for these lines. The test also does a bad job at testing exactly what the difference between these 3 cases is expected to be (it just asserts that the code behaves the way it used to). While we have ample cypress test coverage for the new functionality, and no other cypress tests broke, the exact implementation changed - and I refuse to keep maintaining the implementation twice (in both the straight-forward implementation and the test that describes the implementation almost line-by-line).
…er (just that everything is consistent in the end). Also adding workaround for checkboxes order (as it does not properly support arrays just yet).
These headers is not used by the library, but directly by apps. edit |
|
Yup. I don't know all the details, so I'm leaning on people who do. I also suggested in the linked issue for a backend implementation that this should be optional/opt-in: Altinn/app-lib-dotnet/issues/187 With that said, I can elaborate on the reason I believe those headers should be deprecated; I really want to go in a direction where saving data is more asynchronous than it is right now. Let's say you have a spotty internet connection - I would want saving to occur whenever it is possible (but requiring a stable connection for things like triggers, and actual final submits). Right now, we run save requests all the time, and if one of them fails, the entire app crashes. The assumption that "changing something in one component == one save request" is broken, IMO - and getting rid of it now can let us decouple this code in app-frontend in the future. I've written more about this in #506 and #555. Also here: #339 (comment) |
|
I totally agree with your plans 👍😍. The current semantics is just as annoying to me too. I just think we need to be very careful about breaking backwards compatibility, and preferably have some kind of safety. Maybe we can just search gitea for the names of the headers and open issues in the probably very small number of apps that uses the functionality? |
|
Haha, and it seems like I read the issues in the wrong order. You explained the issue pretty well in in the app-lib-dotnet issue😳 |
|
I did a quick search in my folder of about ~341 apps deployed on prod and tt02, and only 5 of them seem to use the headers at the moment. Or, really just 3 (some were deployed in multiple environments and were counted multiple times because of that). So probably not a big problem. But yeah, I agree that should be wary of breaking backwards compatibility. Though, this won't come into effect unless you upgrade the nuget packages - and we have fairly well-established documentation for what you need to be aware of when upgrading those. |
# Conflicts: # yarn.lock
…d (as discussed recently, to avoid a 404 error on the server)
|
SonarCloud Quality Gate failed. |
Description
Note: This requires backend support, which has not arrived yet. See the related issues for more details.
When saving, we currently pass along a best-effort indication of which component ID performed the operation, but this is not sufficient info for the backend at times, and we can't guarantee that we send in the correct component ID here (because saving to the backend isn't, and shouldn't be tied 1-1 to saving in a component).
The way this works is that is queries a 'feaureset' endpoint in the start of the application runtime (which currently returns a 404 on all backend versions). If that endpoint responds, with an object like
{"multiPartSave": true}, subsequent save operations will be a multipart PUT request with two parts (dataModelandpreviousValues). ThedataModelpart is the same as what we saved before, whilepreviousValueswill be a key-value object of dot-notation keys, along with the previous values for all changed data model paths (ornullif the value did not exist before). The formerX-DataFieldandX-ComponentIdheaders are not passed along in a multipart request, and should be considered deprecated.Related Issue(s)
Backend implementation (currently missing):
Verification
src/layout/layout.d.tsandlayout.schema.v1.json, and these are all backwards-compatible