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

fix: [M3-7811] - Fix Users & Grants query filtering on user_type #10230

Merged

Conversation

mjac0bs
Copy link
Contributor

@mjac0bs mjac0bs commented Feb 26, 2024

Description πŸ“

The filter we're applying to child accounts needs to be scoped to child users. Currently, with the parent/child flag on, all users are seeing an error.

From API: "Because this is a computed filter, the API only supports filtering on a single user_type (and no other complex filtering logic), which in this case would be child."

Changes πŸ”„

  • Updated the filters on the useAccountUsers that populates the table of non-proxy users to only filter on child users when logged into a child or proxy user account. API doesn't support complex filtering logic on this field.
  • Fixed an issue where the numCols was dynamic for the users in the Business Partner table, and it should not have been. (We don't display the Child Account Access column there, as we do in the other table.)

Note

Test coverage for this flow will be completed in M3-7500.

Target release date πŸ—“οΈ

3/5/2024

Preview πŸ“·

Before After
Screenshot 2024-02-26 at 12 51 00 PM Child user: Screenshot 2024-02-26 at 12 41 56 PM Proxy user: Screenshot 2024-02-26 at 1 26 33 PM Parent user: Screenshot 2024-02-26 at 12 47 21 PM Regular user: Screenshot 2024-02-26 at 12 51 17 PM

How to test πŸ§ͺ

Prerequisites

(How to setup test environment)

  • Ensure Parent/Child feature flag is on in dev tools.

Reproduction steps

(How to reproduce the issue, if applicable)

  • Using our dev site, visit the Users & Grants page.
  • Observe that after attempting to load, the Users table fails to populate and displays the above filtering error.

Verification steps

(How to verify changes)

  • Note: You can use the MSW to test these changes locally, but it's a little weird for parent and default users unless you comment out the proxy user being returned from the */users endpoint. Proxy users will only exist on child accounts, so it's never the case that we'd be returning a proxy user for users of parent or default type.
  • Go to serverHandlers.ts and modify the */profile user_type to child. Confirm that both tables load with the expected data (proxy user in the first table, child user(s) in the second).
  • Modify the user_type to proxy. Confirm the same behavior as above.
  • If you want to test parent or child with real API data, contact me if you want me to share my parent/child test account details or follow the provisioning instructions to create your own.

As an Author I have considered πŸ€”

Check all that apply

  • πŸ‘€ Doing a self review
  • ❔ Our contribution guidelines
  • 🀏 Splitting feature into small PRs
  • βž• Adding a changeset
  • πŸ§ͺ Providing/Improving test coverage
  • πŸ” Removing all sensitive information from the code and PR description
  • 🚩 Using a feature flag to protect the release
  • πŸ‘£ Providing comprehensive reproduction steps
  • πŸ“‘ Providing or updating our documentation
  • πŸ•› Scheduling a pair reviewing session
  • πŸ“± Providing mobile support
  • β™Ώ Providing accessibility support

@mjac0bs mjac0bs self-assigned this Feb 26, 2024
@mjac0bs mjac0bs added the Bug Fixes for regressions or bugs label Feb 26, 2024
@mjac0bs mjac0bs marked this pull request as ready for review February 26, 2024 21:38
@mjac0bs mjac0bs requested a review from a team as a code owner February 26, 2024 21:38
@mjac0bs mjac0bs requested review from cpathipa, abailly-akamai and jaalah-akamai and removed request for a team February 26, 2024 21:38
Copy link
Contributor

@jaalah-akamai jaalah-akamai left a comment

Choose a reason for hiding this comment

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

Looks good - thanks for fixing

@jaalah-akamai jaalah-akamai added the Add'tl Approval Needed Waiting on another approval! label Feb 26, 2024
@bnussman-akamai bnussman-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Feb 27, 2024
@mjac0bs mjac0bs merged commit fd9617d into linode:develop Feb 27, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved Multiple approvals and ready to merge! Bug Fixes for regressions or bugs Parent / Child Account
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants