🐛 Remove duplicate targets in advanced options screen

[ibolton336]

Resolves https://issues.redhat.com/browse/MTA-2795

• Resolves any potential duplicates between hardcoded default targets / sources labels & parsed labels from targets.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 10 lines in your changes missing coverage. Please review.

Project coverage is 42.16%. Comparing base (b654645) to head (1c88bea).
Report is 158 commits behind head on main.

Files	Patch %	Lines
...pages/applications/analysis-wizard/set-options.tsx	0.00%	10 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1906      +/-   ##
==========================================
+ Coverage   39.20%   42.16%   +2.96%     
==========================================
  Files         146      166      +20     
  Lines        4857     5319     +462     
  Branches     1164     1292     +128     
==========================================
+ Hits         1904     2243     +339     
- Misses       2939     3060     +121     
- Partials       14       16       +2     

Flag	Coverage Δ
client	42.16% <0.00%> (+2.96%)	⬆️
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.

[sjd78]

LGTM

[sjd78]

Have a look at the filters being used for allTargetLabelsFromTargets and allSourceLabelsFromTargets -- they don't look right.

Those could really be simplied to just:

  xyz = allLabelsFromTargets.filter(
    (label) => getParsedLabel(label?.label)?.labelType === "target" // or "source"
  );

Next, both the defaultTargetsAndTargetsLabels and defaultSourcesAndSourcesLabels can be deduped and sorted the same way for a consistent list experience.

Finally, any insight as to why the TargetLabel names are completely ignored and just a parsed bit of the label is used?

[sjd78]

Actually I think you can just do the same thing as is done for defaultSourcesAndSourcesLabels and then sort based on the label as it is done now.

So more like...

  const defaultTargetsAndTargetsLabels = [
    ...new Set(defaultTargets.concat(allTargetLabelsFromTargets))
  ].sort((t1, t2) => {
    if (t1.label > t2.label) return 1;
    if (t1.label < t2.label) return -1;
    return 0;
  });

Then the existing use of defaultTargetsAndTargetsLabels all stay the same.

[ibolton336]

Looks like this doesn't fix the dupe issue since the objects from the two arrays are constructed separately. Set isn't working how we'd expect here. Need to do the concat before the set I think.

[ibolton336]

Have a look at the filters being used for allTargetLabelsFromTargets and allSourceLabelsFromTargets -- they don't look right.

+1 Just pushed this suggested update.

Finally, any insight as to why the TargetLabel names are completely ignored and just a parsed bit of the label is used?

I think the idea was to keep a mapping back to the names original form so that we have a display value & one that we can pass to the hub when updating the relevant resources.

[sjd78]

LGTM

[ibolton336]

• Solution: Concatenates two arrays and processes them to ensure uniqueness based on the label property by transforming each item into key-value pairs, storing them in a new Map to overwrite duplicates, and then converting the map's values back into an array.

[sjd78]

LGTM -- more obvious what you're doing there now!