✨ Change-the-default-mode

[yael-spinner]

This update introduces a function that changes the default state based on the type of application being deployed, ensuring optimal configuration for different scenarios while maintaining backward compatibility.
Related issue: #1837
Related enhancement PR: #209

[codecov]

Codecov Report

Attention: Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.

Project coverage is 42.09%. Comparing base (b654645) to head (3b430cc).
Report is 241 commits behind head on main.

Files with missing lines	Patch %	Lines
...s/applications/analysis-wizard/analysis-wizard.tsx	85.71%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2106      +/-   ##
==========================================
+ Coverage   39.20%   42.09%   +2.89%     
==========================================
  Files         146      175      +29     
  Lines        4857     5644     +787     
  Branches     1164     1423     +259     
==========================================
+ Hits         1904     2376     +472     
- Misses       2939     3147     +208     
- Partials       14      121     +107     

Flag	Coverage Δ
client	42.09% <85.71%> (+2.89%)	⬆️
server	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rszwajko]

Please check my questions below.

[rszwajko]

Thanks for providing the context for the changes. I have some more comments (see below). I need also verify how useEffect behaves in this place - will comment here when I'm done.

[sjd78]

Looks on the right track, but it can be paired down a bit and still work properly.

[sjd78]

Suggested change

	mode: determineMode(applications),
	mode: determineMode(applications) ?? "source-code-deps",

That way, if a mode can't be figured out from the selected apps, the fallback/default mode is the original default.

[rszwajko]

@sjd78 unfortunately @yael-spinner hit a real bug - the AnalysisWizard gets re-rendered with every change on the applications screen which means that the defaults are being set at start time when no apps are selected.
So useEffect is necessary here as a workaround. The good news is that using (our old friend) key property we can force re-creating the wizard when selected apps change. The following fix worked for me:

iff --git a/client/src/app/pages/applications/applications-table/applications-table.tsx b/client/src/app/pages/applications/applications-table/applications-table.tsx
index d102d69a..d4affbb4 100644
--- a/client/src/app/pages/applications/applications-table/applications-table.tsx
+++ b/client/src/app/pages/applications/applications-table/applications-table.tsx
@@ -1060,6 +1060,7 @@ export const ApplicationsTable: React.FC = () => {
 
         <TaskGroupProvider>
           <AnalysisWizard
+            key={selectedRows.map(({ name }) => name).join()}
             applications={selectedRows}
             isOpen={isAnalyzeModalOpen}
             onClose={() => {
(END)

[sjd78]

Ah right! I forgot about the order state gets loaded up in modals like this. Using the key prop does make sense.

For some reference: https://react.dev/learn/preserving-and-resetting-state#resetting-a-form-with-a-key

So use the key attribute and drop useEffect()?

[yael-spinner]

@sjd78 @rszwajko I changed the code according to your comments

[sjd78]

This looks good. Just put it in a function, iterate through the applications array and get the mode for each application. At the end either all of the modes are the same, or they are different. Same = return that mode, different = return undefined.

[yael-spinner]

I updated the code according to the previous requests and comments, If there are any questions, I'm available for clarification."

[rszwajko]

Looks good! Consider the comment below as nice-to-have but not mandatory. It would be also nice to change the title of this PR to better reflect the change and remove hyphens between words.

[rszwajko]

Note that this could be reduced to problem of removing duplicates i.e.

// using array -> set -> array
const modes =Array.from(new Set(application.map(....
// or using radash
const modes = unique(application.map(...