RHCLOUD-31826: quick fix for sentry errors when switching between roles and groups

[florkbr]

Description

We have an issue where setting filters in the groups UI then switching to the roles UI is causing sentry errors. See attached RHCLOUD ticket and slack discussion here: https://redhat-internal.slack.com/archives/C05M0SNTLM8/p1712174912636909. This is causing several issues in our test suites. The issue is not reproducible every time in my observations, but if you set the filter and click back and forth enough it will eventually error.

I suspect this may have been introduced in #1601. After that change, while we load the roles UI - we make a request to load the adminGroup here: https://github.com/RedHatInsights/insights-rbac-ui/blob/master/src/smart-components/role/roles.js#L96. The adminGroup is then passed into our createRows helpers here: https://github.com/RedHatInsights/insights-rbac-ui/blob/master/src/smart-components/role/roles.js#L182 (which is throwing errors).

AFAIK there is no guarantee the adminGroup request will have loaded by the time the UI finishes loading and calls the createRows helper (hence adminGroup will be undefined until the request finishes and it makes its way into redux).

For now I've simply added an optional chaining check to the line that was throwing the sentry error. I think long term we need a few changes:

1. Do we really need to make the same admin group request every time we load the page? Or can it be cached?
2. If the admin group must be loaded before we do any table evaluation, we need to move the call to request it sooner in the page loading and stop the UI from rendering any further
3. Do we really need to use redux for this?
4. We really should convert this to typescript - which likely would have alerted us to the fact that adminGroup can either be undefined | Object

https://issues.redhat.com/browse/RHCLOUD-31826

---

Screenshots

Before:

After:

---

Checklist ☑️

• PR only fixes one issue or story
• Change reviewed for extraneous code
• UI best practices adhered to
• Commits squashed and meaningfully named
• All PR checks pass locally (build, lint, test, E2E)

• (Optional) QE: Needs QE attention (OUIA changed, perceived impact to tests, no test coverage)
• (Optional) QE: Has been mentioned
• (Optional) UX: Needs UX attention (end user UX modified, missing designs)
• (Optional) UX: Has been mentioned

[codecov-commenter]

Codecov Report

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

Project coverage is 55.09%. Comparing base (703cdd9) to head (bce57af).

❗ Current head bce57af differs from pull request most recent head 72f9286. Consider uploading reports for the commit 72f9286 to get more accurate results

Files	Patch %	Lines
src/smart-components/role/role-table-helpers.js	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1615   +/-   ##
=======================================
  Coverage   55.09%   55.09%           
=======================================
  Files         160      160           
  Lines        3908     3908           
  Branches     1159     1159           
=======================================
  Hits         2153     2153           
  Misses       1589     1589           
  Partials      166      166           

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

[florkbr]

FWIW this change doesn't seem to break the functionality added in 1601:

[fhlavac]

Thank you @florkbr for taking a look, regarding our points:

1-3) that definitely has to be changed. I should have realized that in that PR. In this case, it seems to me that it would be best to download and cache the Default admin access group once, because we only need its ID, which should not changed (I probably switched it in my thoughts with the Default access group, where the ID can be changed through customizations/reset).

4)I fully agree, we are already adding new files in TS and trying to move to TS when doing a larger refactoring somewhere. We should focus on this more during the tech debt sprint, or maybe a hackathon. There is a lot of work to be done

[florkbr]

@fhlavac sounds good! I'll open a ticket and link it here for 1-3 and 4 separately.

[florkbr]

Ticket for 1-3: https://issues.redhat.com/browse/RHCLOUD-32057

I'll take a look in the backlog if I can find one for 4. Otherwise I'll create one.

For now I'm going to merge this to help relieve some of the strain on our tests.