Figure out how to handle duplicate component-ids across layouts

[standeren]

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

[nkylstad]

We should not allow duplicate component ids across layouts. My understanding is that this is also something that is not going to be supported by v4 apps, so we should enforce unique component ids even across layouts.

[nkylstad]

Added the "ux"-label. We would probably need to implement some sort of validation here, and that means we need to have a good way of showing the user that there is some issue with their config. When adding components from Studio, we ensure that id's are unique across all layouts (probably not across layout-sets though, is that also a requirement?), but we cannot control what the service developers do when working on the files directly.

[nkylstad]

We should look at the design that was implemented for when a group contains an invalid child ID. Could we do something similar?

[Annikenkbrathen]

After reviewing this task a bit, I have some questions that should be discussed. I've been thinking about how we can steer clear of this problem, but I've also checked out some ideas on how to fix it in Studio once the duplication has happened

1. Since this "error" cannot be rectified manually in the studio but only through the implementation of an existing file, is the idea that they will be notified when they enter the studio? Are these users using Studio? Perhaps these messages should be communicated elsewhere or in multiple places?
2. Is there a way to prevent this condition from ever occurring?
3. How can studio validate and warn the users of the duplicate before implement the file?

I've thought of what the users might need in this case and summarized some points we should cover.

I have two suggestions for the solution in Studio. In cases like this, it's common practice to choose between replacing the old file or creating a new one with a different name. We can also provide the option to delete the new one and keep the old one.

1. Similar to @nkylstad suggestion. The component is highlighted in red in the column. The user can fix it in the studio whenever they want (must be fixed before deployment - maybe the message should occur on this site too), but other edits are not frozen.

2. A modal that pops up when the users enter Studio after implementation. Here, you have to make a decision about the duplicate before you get access to studio. This forces the users to handle the situation immediately, but could be inconvenient if they want to edit something else first. its similar to how we solve the merge conflict. This could be (also unpractical if the users want to see the component and its in studio before deleting)

In these example sketches, only two options are presented for the users: either delete this component or change the name of the duplicate. How many/ which options should the user have? Should they have the options to:

• Chose which of the two component to delete
• Create a new ID for the duplicate (or make Studio create a new name automaticly)
• accept a suggestion from studio (studio solve it by deleting the newest, or giving the newest a new id)
• Delete both?

[TomasEng]

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.

[framitdavid]

Do we have any thoughts on how often this happens with users? I agree with @TomasEng that it becomes complex to support a separate interface for all possible errors that can occur when manually editing code. However, I still believe that we should consider whether it is appropriate to support what may happen more frequently than others. If we don't have any idea of how often this occurs, I vote for a modal in the first round and gather feedback from users over time to see if we need and should make adjustments or not. 😊

Alternative 1 also feels very aggressive with a lot of errors and perhaps a bit of information overload? Are there only errors with duplicate IDs or are there errors in all accordions as well? (it might look like that for a user)

[Annikenkbrathen]

thanks for quick response 😃

Yes, a modal can be an effective solution for handling conflicts. And what @TomasEng mentioned, that other messages could potentially appear as well, is a good point for choosing a modal. Yes, @framitdavid , I agree that it would be useful to have an idea of how often this occurs. If it doesn't happen frequently during the development of an application, a modal should be sufficient.

Would it be possible to indicate where the duplication occurs in the modal? I think it's important for users to be able to recognize where in the app the problem arose. Whether it's by referring to component names and layout as shown in the sketch, or if we can display it in another visual way.

And then the next question is, what choices should users have?

• Chose which of the two component to delete
• Create a new ID for the duplicate (or make Studio create a new name automaticly)
• accept a suggestion from studio (studio solve it by deleting the newest, or giving the newest a new id)
• Delete both?

[TomasEng]

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.

Would it be possible to indicate where the duplication occurs in the modal? I think it's important for users to be able to recognize where in the app the problem arose. Whether it's by referring to component names and layout as shown in the sketch, or if we can display it in another visual way.

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.

And then the next question is, what choices should users have?

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?

[Annikenkbrathen]

Good point! we can test your suggestion Tomas, and start testing if displaying the user's JSON code directly is good enough. I can try to make a good error message by following to Gørilds tips, that we can use in the modal 😊

[Annikenkbrathen]

[TomasEng]

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:

[Annikenkbrathen]

Den visningen du skisserer her @TomasEng er like fin tenker jeg. Ja nå har jeg ikke helt oversikt over hvilke andre feil som kan dukke opp, men det er jo veldig fint om vi får til at Studio viser alle typer feil. Da kan TE kanskje bruke studio til å validere filen sin 🤗 SÅ da stemmer eg også for å lage en mer universell visning. Feilmeldingsteksten kan vel likevel varieres fra type feil? - Sånn at vi fortsatt kan få klare og forståelige feilmeldinger

[TomasEng]

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.

[framitdavid]

@TomasEng and @Annikenkbrathen I believe it would be beneficial to be able to identify errors directly in the JSON files, especially errors that arise only from manual changes in the code.

Regarding errors that occur in the Studio (for non-coding users), we should be able to display validation errors directly in the interface. Exposing "code/configuration" to users who primarily work with the interface may seem unnecessary complex for them? For example, if I change the name of a property in the data model, this changes does not reflected in the associated components with data model binding. (This is something Studio might handle?). When faced with such situations, it would be advantageous to be able to display a message directly in the Studio informing the user that there is no path to this property in the data model.

Note: It's not possible to encounter a duplicate ID error without manual changes in the code/config. We have already implemented validation in the input field for editing component IDs to prevent duplicates.

[nkylstad]

I'm a little confused as to why the user cannot edit this themselves in the Studio UI? I just tested setting the same component ID for a component on 2 different pages. Nothing crashes in the basic config panel. I know that with expressions this is an issue, but if we inform the user of the issue and ask them to fix it, can they not just update the id themselves manually in Studio? @framitdavid @TomasEng @Annikenkbrathen

[TomasEng]

If this is on two different pages, it might work since they are two separate objects and we are currently not displaying them simultaneously. But that's just one particular case. What if they are on the same page? There is an infinite number of errors that can be done by editing the code manually, so it's impossible to account for all them in the user interface. Therefore I think it is a good rule in general that errors that have been introduced manually also should be fixed manually. @nkylstad

[nkylstad]

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.

[TomasEng]

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.

[nkylstad]

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

• copy a layout without updating id's
• make manual changes to component id The new design for displaying component id might help a little here, as it's a little more subtle. Hopefully most of our users don't need to think about component ids.

[TomasEng]

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.)

[Annikenkbrathen]

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

[Annikenkbrathen]

I'll move this task back to our to-do list. I'm happy to discuss it further when a developer picks it up

[nkylstad]

Closing this as completed.