feat: Provide Edit support in Sources tab for multi-source app (#17588)

[keithchong]

Fixes #17588

This PR builds upon #17275

This PR adds/enables the "Edit" button to the Source and Parameter sections of each collapsible panel under the new Sources tab added from #17275.

There are a few noteworthy design points to this:

• Two Edit buttons are added. One for the 'core' source information including Repo URL, path, etc. See the discussion in the proposal as to why two edit buttons are needed
• The repo details are not loaded until the source panel is expanded. This will reduce 'flickering' if all repo sources were loaded at once, and if the UI needs to be updated if underlying data is changed. This will also help with performance.

• The collapse state of each source panel is 'remembered' if the user traverses the Resource details tabs. It is also recalled if the details sliding panel is closed and opened back up again
• The existing support for the single source field can be easily removed with minimal changes. Did not want to change the behavior of that.
• Validation of a field seems to be broken (even without this change, and on single source apps). It appears there needs to be a change in the argo UI. I added new validation for just the repo URL to show that the validation works:

Testing:

• I tested multiple source apps with 2, 3 and 4 sources. I edited and changed the repo URL or path, and subsequently, the parameters ( the bottom panel) changes to reflect the new source type
• I tested modifying and saving Helm parameter values changes, and the override behavior looks the same compared to using a single source app.
• After saving, the source panel may 'load' several times (due to model changes), but I think that is probably acceptable.
• If you are in Edit mode, and wait for some time, the page will refresh and will kick you out of edit mode. This is similar behavior to what is happening currently with single source apps.

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
• The title of the PR states what changed and the related issues number (used for the release note).
• The title of the PR conforms to the Toolchain Guide
• I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
• I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
• Does this PR require documentation updates?
• I've updated documentation as required by this PR.
• I have signed off all my commits as required by DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My build is green (troubleshooting builds).
• My new feature complies with the feature status guidelines.
• I have added a brief description of why this PR is necessary and/or what this PR solves.
• Optional. My organization is added to USERS.md.
• Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).

[ishitasequeira]

@keithchong , tried testing the PR out. Please find couple of my findings below:

1. The Sources page allows to edit multiple sources at the same time. But, saving one source successfully leads to reload of the entire page which leads to loosing unsaved edits in another source. I think, we should not allow users to edit multiple apps at the same time or add a note somewhere that editing individual source will update only that specific source on the application.

2. The ref field is missing from the expanded source panel.

3. We allow users to edit both upper and lower section of the source. But the lower section does not get updated as soon as we change the source type from upper section to a different type (e.g. Directory to Helm or vice versa). Maybe make the other section non-editable while update one?

[keithchong]

Hi @ishitasequeira , thanks for testing this out.

For point 1, yeah, I only disable the lower or upper Edit button within the one source card. I don't disable all the other source cards' Edit buttons. This is something I will have to address later as a 'fit and finish' issue, because the main path is there.

For point 2, done. Merge issue from stage 1 PR. I also added a span element so it will always show up even if there is no value there. (See ref in the video below)

For point 3, I thought this was working before. Is the following video what you're describing?

I also included a fix for another weird issue where for single source apps only, the Helm type parameters updated fine when changed, but when you save the changes, there were duplicate entries for Values and Value Files n the view.

[keithchong]

@ishitasequeira , I had the mechanism set up so that within one source card, the Edit button is disabled if the other is clicked. The change might have been in my stage 3 branch for Add and Delete, and I didn't move it over for this PR. I've just added the fix here.

[ishitasequeira]

@keithchong it seems the ref field was added but it does not reflect the value when in edit mode. Added a recording of the same.

Screen.Recording.2024-04-26.at.4.51.05.PM.mov

[keithchong]

Thanks for testing this. I've updated it. Looks like some other fields were not updated as well so those changes are now included.

[ishitasequeira]

Hey @keithchong , thanks for fixing this. I found another thing.

For the source with the ref field, I can see the Plugin parameters with default values in the lower section even if they do not seem to make sense for the source.

[ashutosh16]

Not very much familiar with dataloader but don't we need DataLoader? how you are passing the app

[keithchong]

Hi @ashutosh16 , thanks for reviewing this. In my first stage change (for showing all the sources in read-only mode, with the Edit button disabled), this DataLoader is used to load all the sources all at once. See the call to getSources function, which I've now removed below. It calls the appDetails api to get all the details of the sources. If I left it this way, that means that once you visit the Sources (formerly the Parameters) tab, all the data is loaded all at once for all the collapsible sections, regardless if they are collapsed or not. Similarly, even before any of my changes, for the single source case, it loads once you visit the tab.

With this new approach, I delayed the loading for each source until the collapsible section is expanded. This improves performance and avoids flickering when any change in the sources occur. The DataLoader code is moved to application-parameters.tsx. For multiple sources, see line 250. For the single source, see line 192. This explains my second bullet point here: #17890 (comment)
The Loading message appears within each source card, and not outside all the cards.

Note that I was not able to reduce all the flickering/refreshing that is happening, when the underlying repo details is being changed, and the components need to be updated, but it appears to be acceptable and I think it is the same as if the single source is being updated too.

Note that I refactored it so that when the single source field is 'deprecated' or 'removed', we could more easily remove the code related to how the single source is handled (even if there is a bit of duplication). Also, I tried to minimize any potential regressions against the original single source behavior.