altinn-studio: Figure out how to handle duplicate component-ids across layouts
Description
…
NB: The decision will also play a role in the dynamics editor since it is important that it is not ambigious which component the expression refers to. Since discovered duplicate component ids while working with frontend-test app while developing the dynmaics editor, a temporary solution was implemented to visualize this. If we figure out a way to avoid that this state will ever occur, this implementation should be removed. It is marked by this issue number in code.
We should also look into how to handle this in the expression editor to avoid page crash.
Suggested approach (see comments below for more details):
-
Implement updated validation logic that checks uniqueness against all components across layouts, not just within the current page…
-
when copying a layout, Studio automatically add a suffix or something to the ID(we don’t have support for copying layouts in Studio yet, so this one is not relevant right now) -
When it happens to be duplicate IDs across layouts. A Modal window opens and warns users about the issue. This modal can provide clear information about the problem and suggest solutions, such as changing the IDs or deleting one of the duplicated components.
Proposed solution
-
when copying a layout, Studio automatically add a suffix or something to the ID
-
When it happens to be duplicate IDs across layouts. A Modal window opens and warns users about the issue. This modal can provide clear information about the problem and suggest solutions, such as changing the IDs or deleting one of the duplicated components.
-
When it happens to be a duplicate ID on the same page, or an other issue that have to be fixed manually, a Modal window opens and warns users about the issue. We show them the error in the code and ask them to fix it manually
-
When it happens to be a duplicate ID on the same page, or an other issue that have to be fixed manually, a Modal window opens and warns users about the issue. We show them the error in the code and ask them to fix it manually
### Tasks
- [ ] https://github.com/Altinn/altinn-studio/issues/12479
- [ ] https://github.com/Altinn/altinn-studio/issues/12634
- [ ] https://github.com/Altinn/altinn-studio/issues/12635
About this issue
- Original URL
- State: open
- Created 10 months ago
- Comments: 22 (13 by maintainers)
Yep - it used to be only relevant within a layout, but with expressions we now need to enforce uniqueness accross layouts. Therefore we also need a way to inform the users if they have this issue. Not actually sure how big of an issue it actually is, and how many users actually change their ids.
When adding a new component and generating the component id, we do check id uniqueness across all layouts. So it only happens if the users
Ja, vi får kontroll på hva som skal valideres og teksten som skal vises. Vi bør nok også ha en lsite over filer med feil i, siden det kan være flere.
Det var akkurat noe slikt jeg tenkte på, @Annikenkbrathen - nå blir det tydelig for brukeren hva som må endres i koden.
Jeg lurer bare litt på hva vi skal gjøre hvis vi vil utvide dette til andre typer feil? Kunne vi laget en mer universell visning, litt som i et vanlig kodeverkøy, som viser alle feilene? Eller skal vi heller gå ut fra at brukerne finner det meste ved hjelp av andre metoder, som f.eks. JSON Schema-validering? @nkylstad @framitdavid
Jeg har laget en eksempelskisse for å vise hva jeg mener:
Okay so if I understood this correct 😄
Can we:
Implement updated validation logic that checks uniqueness against all components across layouts, not just within the current page…
when copying a layout, Studio automatically add a suffix or something to the ID
When it happens to be duplicate IDs across layouts. A Modal window opens and warns users about the issue. This modal can provide clear information about the problem and suggest solutions, such as changing the IDs or deleting one of the duplicated components.
When it happens to be a duplicate ID on the same page, or an other issue that have to be fixed manually, a Modal window opens and warns users about the issue. We show them the error in the code and ask them to fix it manually
When it comes to copying a layout, I see that it becomes tricky, but what about automatically adding a suffix or something to the ID’s? Just like Windows does when copying files?
When changing an ID, I believe that we could just extend the validation that already exists, so that it checks on all ID’s in the layout set? (Edit: I see now that you had the same suggestion.)
Okay, I didn’t know that this was actually possible in Studio. But that sounds like a bug to me. It shouldn’t be possible to make invalid apps in Studio. Ideally users shouldn’t have to think about component ID’s at all unless if they are working with the code manually.
Those could potentially be 2 separate validations, with different outcomes/actions required from the user. In the case of duplicate id’s on the same page, I agree that that can only happen with manual changes, and may require the user to make manual changes, since we can’t really determine which component is which. In that specific case, the manual route might be an ok place to start.
This issues focus is on the other case though, where we have allowed duplicate id’s across layouts.
The idea that a component ID must be unique on a single page is relatively intuitive, and something that we control for in our tool. However, the idea that component IDs must be unique across different layouts might not be so obvious, and is also not something that we enforce - our validation today only checks uniqueness within the current layout. Therefore it is completely possible today to enter the same id for 2 components in Studio, as long as they are on different pages. However this causes problems in both app and Studio.
It is also a use case that service developers copy layouts and make small adjustments to the new page instead of building an entire new page from scratch in cases where two pages are quite similar. Updating the id’s might not be something one considers in this case, and they might even make their adjustments in Studio and not manually.
We therefore can’t assume that duplicate id’s across different layout pages are the result of manual editing of the files, and forcing the user to make their changes manually in this case goes against our goals with this tool.
UPDATE: As a minimum I think we should update our validation of component id’s to check uniqueness agains all components within the layoutset. I also think the modal route warning the user of the issue may be a good start.
As a start, I think it’s best to just describe the error and let the user handle it manually. Since this error can only be caused by editing the files manually, it’s reasonable to expect that users know how to do it.
The problem with visualizing this is that the error occurs in the very same data structure that we use to render the interface. Therefore I think it is best not to render any layout visualization at all as long as there are errors. For the particular case of duplicate ID’s across layouts I think it’s rather straightforward, but I believe it will be difficult to scale if we in the future want to display several layouts at once. So I think we should start by just displaying a message that tells the user which ID that causes problems. Then we can gather feedback later and consider if we need to expand this functionality, like @framitdavid suggests.
Again, I think this is out of scope of what Studio should be able to do. I am in favor of adding detailed error messages that tells the users how to fix the error manually rather than fixing it automatically. There might be reasons for the duplication that only the users know about, so I think it’s better to give them complete control over the data by letting them handle the errors themselves. Making such “smart” functions work perfectly is difficult.
I have another suggestion that I think would be easier to implement if we want to scale this to other kinds of errors. How about displaying the user’s JSON code directly with highlighting of the errors, together with a link that goes straight to the erroneous file(s) in Gitea?
I think a modal would be the best solution here. We depend on the ID’s just as much as the apps do. I don’t think there is any good way to visualize the layout without relying on the ID’s. How can we know which component to show where? It could be easy enough when they are in separate layouts, but what if they are in the same one?
And how about other kinds of errors, like inexisting ID’s, circular references or invalid multipage group syntax? Or combinations of these? ID duplication is just one of many things that can lead to errors when editing the files manually. Therefore I think creating interfaces for specific errors that are introduced by the users manually is a dangearous path to go. It does improve the user experience indeed, but I don’t think it’s worth the effort since it only applies to users that have already chosen to edit the files manually. We should rather just show them a message to help them fix the error in their code.