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

feat(admin): add user role management #4248

Merged
merged 109 commits into from
Jan 17, 2025
Merged

Conversation

chinook25
Copy link
Contributor

@chinook25 chinook25 commented Sep 23, 2024

What are the main changes you did:

how to test:

Other thing that work:

  • User creation/deletion
  • Updating user password, disabling users
  • Showing tokens

Not implemented: token management

Other tests:

  • go to localhost:3000/admin
  • see that your redirected to the login screen

todo:

  • updated docs in case of new feature
  • added/updated tests
  • added/updated testplan to include a test for this fix, including ref to bug using # notation

@chinook25 chinook25 self-assigned this Sep 26, 2024
Comment on lines 49 to 76

const users = ref<IUser[]>([]);
const userCount = ref(0);
const totalPages = ref(0);
getUsers();

function updateCurrentPage(newPage: number) {
currentPage.value = newPage;
getUsers();
}

const users = computed(() => data.value?.data._admin.users || []);
const userCount = computed(() => data.value?.data._admin.userCount ?? 0);
async function getUsers() {
const { data, error } = await useFetch<IAdminResponse>("/api/graphql", {
method: "post",
body: {
query: `{ _admin { users(limit: ${LIMIT}${offset.value}) { email, settings, {key, value} } userCount } }`,
},
});
if (error.value) {
console.log("Error loading users: ", error.value);
// todo handle error see catalogue error page
}
users.value = data.value?.data._admin.users || [];
userCount.value = data.value?.data._admin.userCount ?? 0;
const divided = userCount.value / LIMIT;
totalPages.value =
userCount.value % LIMIT > 0 ? Math.floor(divided) + 1 : divided;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this should should work, but by keeping the useFetch at the page level and using the 'refresh' or computed there is no need for a separate getUsers func and the setup code . This has the added value of exposing the dat, status, error refs at the page level

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds cool, didn't get it to work that way though with the pagination.

@@ -9,6 +9,8 @@
<!-- <SearchBar /> -->
</div>

<slot name="admin" />
Copy link
Contributor

Choose a reason for hiding this comment

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

why a admin slot , suggest to have slots names related to parts of the header ( left , right , info , cart ) instead of role

Copy link
Contributor

Choose a reason for hiding this comment

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

suggest to add menu item

apps/ui/pages/admin.vue Outdated Show resolved Hide resolved
@chinook25 chinook25 marked this pull request as ready for review January 9, 2025 10:31
@chinook25 chinook25 requested a review from connoratrug January 9, 2025 10:31
Copy link
Contributor

@connoratrug connoratrug left a comment

Choose a reason for hiding this comment

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

approve as wip only , ui / ux setup is not inline with new design
Suggest to align with design and code standerd before merge

Copy link
Member

@mswertz mswertz left a comment

Choose a reason for hiding this comment

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

functionally approved. I am also happy to keep this one open in next sprint to make one round of improvements (e.g. on the in table buttons and using the full screen modal)

Copy link
Contributor

@dtroelofsprins dtroelofsprins left a comment

Choose a reason for hiding this comment

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

Tested:

  • Overview with users and their roles within schemas
  • Create user => can enter name and password, then add user button gets grey, can be pushed, but nothing happens, Close buttons keeps yellow but also does nothing => user does not end up in the list
  • Adjust password of a user NOT TESTED
  • Update existing role within a schema
  • Add new role within a schema
  • Delete a role within a schema
  • Add NEW user via settings within a schema gives error (add existing user works):
    image
  • Remove users from settings on schema level => removed from Admin tools list

At first I thought that updating /adding new roles did not work, but then I found out that I first need to push the PLUS before saving.

Copy link
Contributor

@dtroelofsprins dtroelofsprins left a comment

Choose a reason for hiding this comment

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

Tested:

  • General overview with users and their roles within schemas, including how many tokens they have
  • Create user
  • Edit user
  • Disable user => DELETES THE USER!
  • Adjust password of a user
  • Remove token of a user => can remove, edit screen shows no token anymore, but when going to the overview it still show Tokens(1). On user level the token is also still there.
  • Removing the token by the user also still show Tokens(1) in the overview, and an empty line (with trash button) in the edit screen.
  • Update existing role within a schema
  • Add new role within a schema
  • Delete a role within a schema
  • Add NEW user via settings within a schema => Visible in Admin tools overview
  • Remove users from settings on schema level => removed from Admin tools list

At first I thought that updating /adding new roles did not work, but then I found out that I first need to push the PLUS before saving. This is perhaps not that intuitive.

Copy link
Contributor

@dtroelofsprins dtroelofsprins left a comment

Choose a reason for hiding this comment

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

Tested:

  • General overview with users and their roles within schemas, including how many tokens they have
  • Create user
  • Edit user
  • Disable user
  • Adjust password of a user
  • Remove token of a user => can remove, edit screen shows no token anymore, but when going to the overview it still show Tokens(1). On user level the token is also still there. WON'T DO => separate ticket
  • Removing the token by the user also still show Tokens(1) in the overview, even after re-login and private window, so seems not to be caching.
  • Update existing role within a schema
  • Add new role within a schema
  • Delete a role within a schema
  • Add NEW user via settings within a schema => Visible in Admin tools overview
  • Remove users from settings on schema level => removed from Admin tools list

Regarding the tokens, if this functionality will not be implemented (separate ticket), perhaps it's better to remove the column from the overview, as now it can give false-positive information.

@chinook25 chinook25 merged commit cb3a46e into master Jan 17, 2025
1 check was pending
@chinook25 chinook25 deleted the feat/admin-role-management branch January 17, 2025 08:13
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.

4 participants