Org page

[rmunn]

Fixes #779.

Add an org page, parallel to the project page (and reusing several of its components), but with a slightly different layout, using tabs for the main body of the page.

Work done:

• Extracted common code from project page for header, details list (created date, description, etc.)
• Extracted members list code into common component
  • Currently hard-coded to use ProjectRole
• Edited MembersList component to use a generic role type that can be filled by either ProjectRole or OrgRole
• Used MembersList component in org page
• MembersList component now has a filter bar thanks to @myieye
• MembersList component now can switch between badge list and table view
• Separated MembersList component into project- and org-specific versions; org-specific one is now called OrgMemberTable
• MembersList component is back to using just ProjectRole, simplifying its code again
• Add "Add / Invite Member" button to table view so org page can use it

Deferred to other issues/PRs:

• Add "Bulk add / invite" button to table view so org page can use it
• Implement projects tab on org page (needs backend work so orgs can own projects)
• Implement settings tab on org page (will be separate issue)
• Implement history view on org page, or remove history tab if we're not implementing it anytime soon

Screenshot:

Previous screenshots:

1. Click to view
2. Click to view
3. Members list view
4. Members table view

[github-actions]

UI unit Tests

11 tests   11 ✅  0s ⏱️
 3 suites   0 💤
 1 files     0 ❌

Results for commit c9240d6.

♻️ This comment has been updated with latest results.

[github-actions]

C# Unit Tests

52 tests  ±0   52 ✅ ±0   5s ⏱️ -1s
10 suites ±0    0 💤 ±0 
 1 files   ±0    0 ❌ ±0 

Results for commit c9240d6. ± Comparison against base commit eefe404.

♻️ This comment has been updated with latest results.

[rmunn]

@hahn-kev @myieye - I'd like feedback on the members list component I just extracted. See PR description for screenshot; currently it looks and functions just like the members list for projects, except that I haven't yet implemented the "Add / Invite Member" button. (Or the bulk-add button). My concern, though, is as follows.

Some orgs might be small ("Language Name Here dictionary team") while others might be large (SIL). For large orgs, I think we'd want a member list layout more similar to the users tab on the admin page, but the MembersList component is quite useful for small orgs. Perhaps we automatically switch between them based on member count, or perhaps we provide a UI toggle for org admins to choose which view we want.

[rmunn]

@myieye - Commit 3d93f26 reintroduces using roletext.toLowerCase() in the popup notification, but the role name for orgs is currently "user" and I had set the translated text to "member". Now the notification doesn't match what's in the role select choices. Should I change the translated role name to "User" rather than "Member" for orgs?

[rmunn]

I've now implemented an AddOrgMember button. Chose to copy the AddProjectMember button rather than refactor it to be in common, because the refactor would (IMHO) have added quite a lot of complexity to the button for not that much gain. But I can go back and refactor the two buttons to have common code if it turns out to be a good idea.

[rmunn]

Org page now has a users table where you can add and remove members. Currently adding is done with a button in the table header, and removing is done via a dropdown:

It feels a little cramped: I think there needs to be a bit of vertical space between the tabs and the table, and maybe some space above the tabs. But this is shaping up to be a functional, if still very basic, page. It might be time to call this done and move to the next task: allowing admins to create orgs, and/or allowing orgs to own projects.

[rmunn]

Lots of merge conflicts since I've been on this PR for a while, and the develop has gotten some major work merged recently. But the design work is finished. What's needed next is a bit of polishing of the UI (a bit more vertical space between the org details and the tab view, for example) and implementing any last-minute suggestions. But I'd say this is ready for a review from both the code and UI design perspectives.

[hahn-kev]

@rmunn can you merge develop into this branch before we review is since it's so far behind?

[rmunn]

@rmunn can you merge develop into this branch before we review is since it's so far behind?

Actually that's a good idea. When I run git mergetool and it autoresolves conflicts, it's usually correct, but I've sometimes found that manual correction of its resolution is needed. So by merging develop in before review, there will be more eyes looking at it in case I miss some merge-resolution mistake. Working on that now.

[rmunn]

My first attempt to merge develop into this branch went quite badly wrong as Git got confused by an earlier merge-and-revert I had done, and ended up choosing wrong branch resolutions all over the place. I've been able to rebase the branch to get rid of the merge-and-revert, and now Git is far less confused — and has only a single merge conflict, in the admin dashboard page. (I moved the users table to a separate component, while meanwhile Tim made a two-line change in the users table on the admin dashboard).

I should be able to resolve that merge conflict by hand, without having to merge develop into this branch.

[rmunn]

@myieye - Many comments addressed, but ran out of time to do all of them. Will finish Monday.

[rmunn]

I started implementing the "invite org member by email" flow and realized that it's going to take a bit more effort than I thought it would: we can't simply reuse the existing invite project member by email without changes. We'll need to change the acceptInvitation page to look for a list of orgs to join as well as projects to join, we'll need to edit the invitation JWT to optionally include a list of orgs as well as a list of projects...

I think it's best if I push that work off to a separate PR, so that the org page work can be completed. So for now, I'll leave the "TODO: Implement inviting user" comment in OrgMutations.cs, as a reminder for us to do that work. (I'll also create an issue for it).

[rmunn]

I looked into Scribe, and it requires either a Chrome or Edge extension. I uninstalled Chrome from my Linux computer ages ago because it's getting increasingly privacy-hostile (I use Chromium if I need to test something in Chrome), so I really don't want to reinstall it just for this. @myieye, could I ask you to create an Add_Org_Member scribe similar to the Add_Project_Member one?

[rmunn]

Only problem remaining is the Typescript errors that result from differing definitions of User/TableUser/etc. I don't have a good idea for how to fix them (my earlier attempts have led to much confusion), but perhaps I'm simply missing something obvious. @myieye, could I ask you to take a look and see if you spot something I missed? Run cd frontend; pnpm check and you'll get several errors along the lines of "Argument of type 'Pick<User, "id" | "name" | "email" | "username" | "isAdmin" | "emailVerified" | "locked">' is not assignable to parameter of type 'User'".

[rmunn]

It looks to me like the TableUser type in frontend/src/lib/components/Users/UserTable.svelte is at the center of the Typescript issues I'm running into. If I simply define it as User then some of the errors go away, but I still get things like:

/home/rmunn/code/lexbox/frontend/src/routes/(authenticated)/admin/+page.svelte:159:12
Error: Type '{ id: string; name: string; email?: string | null | undefined; username?: string | null | undefined; isAdmin: boolean; emailVerified: boolean; createdDate: string | Date; locked: boolean; ... 4 more ...; projects: { ...; }[]; }[]' is not assignable to type 'User[]'.
  Type '{ id: string; name: string; email?: string | null | undefined; username?: string | null | undefined; isAdmin: boolean; emailVerified: boolean; createdDate: string | Date; locked: boolean; ... 4 more ...; projects: { ...; }[]; }' is missing the following properties from type 'User': organizations, usersICreated (ts)

I'm too close to the problem to be able to see it clearly any more. I think I need input from someone who hasn't been staring at this code for a month. :-)

[myieye]

@rmunn I just made a decently sized push:

• 461b789 Fixed the TypeScript errors
• 11cf0b9 maybe shouldn't be in this PR, but I was having lots of bizarre problems with SK and updating fixed them 🙃
• 4faa6df is the big one. I did several things:
  • I realized that some components were still generalized for handling org or project roles or verbose vs non-verbose texts, but that the generalisations weren't needed anymore
  • I moved a handfull of components back to the page that their page.ts/.svelte is in, because it makes them easier to find and in some cases there were still imports going back to the page files, so this cleaned them up
  • I've adaopted the practice of exporting some types from page.ts that the svelte page and components can share (e.g. a page-specific User type or OrgUser type etc.)
  • I made your Org role display name component work the same way as the project role display name component
  • I standardized some texts on the org page to be more similar to the project page

[rmunn]

@rmunn I just made a decently sized push:

• 461b789 Fixed the TypeScript errors

🎉 I knew there would be an easy solution that I was just too close to the code to see. Thanks!

I've done a self-review and found one bug, so I pushed commit c9240d6 to fix it. Everything else looks good to me. I think this is finally ready.

@hahn-kev, you have the last remaining "Changes requested" review. Do you want to give this another look?

[hahn-kev]

a couple minor things, then this can be merged in. I also noticed a bit of a permissions issue so I created #879 to track that.

[myieye]

I propose you merge this PR right now and finish the TODOs in the scope of this new issue I created:
#881

[rmunn]

I propose you merge this PR right now and finish the TODOs in the scope of this new issue I created: #881

I'd already implemented the settings tag change before you said that, but I hadn't committed it yet. I was thinking of pushing that commit then asking for thoughts, but instead I'll go ahead and make that commit part of the new branch for #881.

I'll dismiss Kevin's review since you've created the issue to track those remaining tasks, and merge this.

Dismissing as per Tim and Chris's suggestion; remaining work tracked at #881