Skip to content

Query filtering by user group #483

New issue

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

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

Already on GitHub? # to your account

Draft
wants to merge 19 commits into
base: main
Choose a base branch
from
Draft

Query filtering by user group #483

wants to merge 19 commits into from

Conversation

akintner
Copy link
Collaborator

@akintner akintner commented Apr 2, 2025

PULL REQUEST

Summary

Logged-in users should be able to run only the selection of saved queries for a group to which they belong. As per design discussion, super admins will see all queries both in the query library, and at the point of dropdown filtering for running saved queries.

Fixes #436

Checklist

  • Descriptive Pull Request title
  • Link to relevant issues
  • Provide necessary context for design reviewers
  • Ensure test coverage is above agreed upon threshold
  • Update documentation

@akintner akintner marked this pull request as draft April 2, 2025 20:32
Copy link

github-actions bot commented Apr 2, 2025

Coverage report for ./query-connector

Caution

Test run failed

St.
Category Percentage Covered / Total
🔴 Statements
58.61% (-2.09% 🔻)
1709/2916
🔴 Branches
49.85% (-2.03% 🔻)
484/971
🟡 Functions
61.12% (-2.91% 🔻)
349/571
🔴 Lines
58.64% (-1.99% 🔻)
1616/2756
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟡
... / usergroup-management.ts
75.94% (-2.67% 🔻)
65.08% (-1.59% 🔻)
87.1% (-3.23% 🔻)
76.11% (-2.78% 🔻)
🔴
... / user-management.ts
58.65% (-2.95% 🔻)
38.71% (+6.45% 🔼)
85.71% (-6.59% 🔻)
58.33% (-2.96% 🔻)
🔴
... / lib.ts
5.8% (-1.25% 🔻)
0% 0%
5.8% (-1.25% 🔻)
🟢
... / page.tsx
85.88% (-14.12% 🔻)
63.89% (-27.78% 🔻)
93.55% (-6.45% 🔻)
84.81% (-15.19% 🔻)
🟢
... / QuerySelection.tsx
82.61% (-0.15% 🔻)
71.43% (-19.48% 🔻)
83.33% (+8.33% 🔼)
82.22% (+0.08% 🔼)
🔴
... / QueryLibrary.tsx
29.41% (-50% 🔻)
0% (-100% 🔻)
0% (-58.33% 🔻)
31.25% (-50% 🔻)
🔴
... / utils.tsx
23.33% (-6.67% 🔻)
0% (-12.5% 🔻)
0% (-12.5% 🔻)
26.92%
🔴
... / deleteModal.tsx
36.36% (-9.09% 🔻)
0% (-50% 🔻)
0% (-33.33% 🔻)
50%
🟢
... / Modal.tsx
100%
66.67% (-33.33% 🔻)
100% 100%

Test suite run failed

Failed tests: 7/161. Failed suites: 4/21.
  ● User Group and Query Membership Tests › should add multiple users to a group

    TypeError: Cannot read properties of undefined (reading 'id')

      137 |
      138 |     expect(result.totalItems).toBe(2);
    > 139 |     expect(result.items.some((user) => user.id == TEST_USER_1_ID));
          |                                             ^
      140 |     expect(result.items.some((user) => user.id == TEST_USER_2_ID));
      141 |
      142 |     const members = (await getAllGroupMembers(TEST_GROUP_ID)).items;

      at id (src/app/tests/integration/usergroup-management.test.ts:139:45)
          at Array.some (<anonymous>)
      at Object.some (src/app/tests/integration/usergroup-management.test.ts:139:25)

  ● User Group and Query Membership Tests › should add a single user to a group

    TypeError: Cannot read properties of undefined (reading 'id')

      150 |     const result = await addUsersToGroup(TEST_GROUP_ID, [TEST_USER_3_ID]);
      151 |     expect(result.totalItems).toBe(1);
    > 152 |     expect(result.items[0].id).toContain(TEST_USER_3_ID);
          |                            ^
      153 |     expect(result?.items[0].userGroupMemberships?.[0].membership_id).toContain(
      154 |       TEST_GROUP_ID,
      155 |     );

      at Object.id (src/app/tests/integration/usergroup-management.test.ts:152:28)

  ● User Group and Query Membership Tests › should remove multiple users from a group

    TypeError: Cannot read properties of undefined (reading 'id')

      178 |     ]);
      179 |
    > 180 |     expect(result.items[0].id).toContain(TEST_USER_1_ID);
          |                            ^
      181 |     expect(result.items[1].id).toContain(TEST_USER_2_ID);
      182 |
      183 |     const updatedMembers = await getAllGroupMembers(TEST_GROUP_ID);

      at Object.id (src/app/tests/integration/usergroup-management.test.ts:180:28)


  ● User Group and Query Membership Tests › should retrieve user group memberships

    error: syntax error at end of input

      39 | 
      40 |     `;
    > 41 |     await dbClient.query(insertUsersQuery, [TEST_USER_1_ID, TEST_USER_2_ID]);
         |     ^
      42 |
      43 |     // Insert test group
      44 |     const insertGroupQuery = `

      at node_modules/pg-pool/index.js:42:11
      at Object.<anonymous> (src/app/tests/integration/usergroup-query_filtering.test.ts:41:5)

  ● User Group and Query Membership Tests › should add multiple queries to a group

    error: syntax error at end of input

      39 | 
      40 |     `;
    > 41 |     await dbClient.query(insertUsersQuery, [TEST_USER_1_ID, TEST_USER_2_ID]);
         |     ^
      42 |
      43 |     // Insert test group
      44 |     const insertGroupQuery = `

      at node_modules/pg-pool/index.js:42:11
      at Object.<anonymous> (src/app/tests/integration/usergroup-query_filtering.test.ts:41:5)


  ● tests the query building steps › renders the default state

    TestingLibraryElementError: Unable to find an element with the text: Query Library. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.

    Ignored nodes: comments, script, style
    <body>
      <div>
        <div>
          <div
            class="overlay"
          >
            <div
              class="spinner"
            />
            <div
              class="margin-left-1-important page-title"
            >
              Loading...
            </div>
          </div>
        </div>
      </div>
    </body>

      55 |     });
      56 |
    > 57 |     expect(screen.getByText("Query Library")).toBeInTheDocument();
         |                   ^
      58 |
      59 |     expectedQueryNames.forEach(async (name) => {
      60 |       expect(screen.getByText(name)).toBeInTheDocument();

      at Object.getElementError (node_modules/@testing-library/dom/dist/config.js:37:19)
      at node_modules/@testing-library/dom/dist/query-helpers.js:76:38
      at node_modules/@testing-library/dom/dist/query-helpers.js:52:17
      at node_modules/@testing-library/dom/dist/query-helpers.js:95:19
      at Object.getByText (/home/runner/work/dibbs-query-connector/dibbs-query-connector/query-connector/src/app/(pages)../../../../../../queryBuilding/page.test.tsx:57:19)


  ● tests the query building steps › renders

    Unable to find an element with the text: Start with Query Builder. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible.

    Ignored nodes: comments, script, style
    <body>
      <div>
        <div>
          <div
            class="overlay"
          >
            <div
              class="spinner"
            />
            <div
              class="margin-left-1-important page-title"
            >
              Loading...
            </div>
          </div>
        </div>
      </div>
    </body>

      27 |
      28 |     // Wait for the asynchronous data to resolve and the component to update
    > 29 |     await waitFor(() => {
         |                  ^
      30 |       expect(screen.getByText("Start with Query Builder")).toBeInTheDocument();
      31 |       expect(screen).toMatchSnapshot();
      32 |     });

      at waitForWrapper (node_modules/@testing-library/dom/dist/wait-for.js:163:27)
      at Object.<anonymous> (/home/runner/work/dibbs-query-connector/dibbs-query-connector/query-connector/src/app/(pages)../../../../../../queryBuilding/QueryBuilding.test.tsx:29:18)

Report generated by 🧪jest coverage report action from 1ecd91a

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 69.23077% with 20 lines in your changes missing coverage. Please review.

Project coverage is 60.35%. Comparing base (0b06832) to head (99dccc0).

Files with missing lines Patch % Lines
...s)/queryBuilding/querySelection/QuerySelection.tsx 54.05% 16 Missing and 1 partial ⚠️
query-connector/src/app/backend/user-management.ts 70.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #483      +/-   ##
==========================================
- Coverage   60.65%   60.35%   -0.31%     
==========================================
  Files          94       94              
  Lines        3833     3879      +46     
  Branches      849      879      +30     
==========================================
+ Hits         2325     2341      +16     
+ Misses       1503     1454      -49     
- Partials        5       84      +79     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Only show queries attached to user's teams in query flow
3 participants