Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use dynamic export instead of import for GroupingsTable #110

Merged
merged 1 commit into from
Nov 21, 2024

Conversation

JorWo
Copy link
Contributor

@JorWo JorWo commented Nov 14, 2024

Ticket Link

GROUPINGS-1855

List of squashed commits

  • Adds missing test for groupings-table-skeleton.tsx
  • Fixes React key warning in groupings-table-skeleton.tsx
  • Fixes incorrect text and background color for copy tooltip in grouping-path-cell.tsx

Test Checklist

  • Unit Tests Passed
  • General Visual Inspection

@JorWo JorWo force-pushed the dev-jordanw4-1855 branch 2 times, most recently from 2319ce4 to bdb492a Compare November 14, 2024 23:06
@JorWo JorWo requested a review from mhodgesatuh November 14, 2024 23:06
Copy link
Member

@mhodgesatuh mhodgesatuh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some users disable cookies. Has this been tested to see how this is handled to ensure nothing uncontrolled happens.

@JorWo
Copy link
Contributor Author

JorWo commented Nov 14, 2024

Some users disable cookies. Has this been tested to see how this is handled to ensure nothing uncontrolled happens.

Good point. I just tested running the site without cookies and the user is unable to make it past the CAS login anyways. This is the case for both prod and test CAS environments. I think the Duo 2FA needs cookies.

Copy link
Member

@mhodgesatuh mhodgesatuh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your response addresses the cookie issue as far as I'm concerned.

@JorWo
Copy link
Contributor Author

JorWo commented Nov 14, 2024

I also tested the scenario when the user disabled cookies after logging in. Upon clicking anything, the user gets redirected to the home page, signed out.

@JorWo
Copy link
Contributor Author

JorWo commented Nov 18, 2024

Will wait for completion of Prayag's and Ralph's PR for Admin page before merging.

@JorWo JorWo force-pushed the dev-jordanw4-1855 branch from bdb492a to 860037f Compare November 21, 2024 19:39
@JorWo JorWo changed the title Replace localStorage with cookies in GroupingsTable Use dynamic export instead of import for GroupingsTable Nov 21, 2024
@JorWo JorWo force-pushed the dev-jordanw4-1855 branch from 860037f to 56b76d3 Compare November 21, 2024 19:47
@JorWo JorWo force-pushed the dev-jordanw4-1855 branch from 56b76d3 to 1a6cfa0 Compare November 21, 2024 19:48
@JorWo JorWo merged commit c88af0e into uhawaii-system-its-ti-iam:main Nov 21, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants