feat: Add project select to the contributors page gf-504 #516
Conversation
GvoFor
requested review from
what1s1ove,
liza-veis and
yevhenii-hladkov
as code owners
…not only selected gf-504
apps/backend/src/modules/contributors/contributor.controller.ts
Outdated
Show resolved
Hide resolved
| const { onPageChange, onPageSizeChange, page, pageSize } = usePagination({ | ||
| queryParameterPrefix: "contributor", | ||
| totalItemsCount: totalCount, | ||
| }); | ||
|
|
||
| useEffect(() => { | ||
| const loadContributors = useCallback(() => { |
apps/backend/src/modules/contributors/contributor.controller.ts
Outdated
Show resolved
Hide resolved
apps/backend/src/modules/contributors/contributor.repository.ts
Outdated
Show resolved
Hide resolved
| } from "../../../../libs/types/types.js"; | ||
| import { type ContributorOrderBy } from "../enums/contributor-order-by.enum.js"; | ||
|
|
||
| type ContributorGetAllRequestDto = { |
There was a problem hiding this comment.
Aren't these query parameters? Request dto is for request body
type ContributorGetAllQueryParameters =
apps/backend/src/modules/contributors/contributor.controller.ts
Outdated
Show resolved
Hide resolved
| switch (orderBy) { | ||
| case ContributorOrderBy.CREATED_AT: { | ||
| query.orderBy(ContributorOrderBy.CREATED_AT, SortType.DESCENDING); | ||
| break; | ||
| } | ||
|
|
||
| public async findAllWithoutPagination({ | ||
| contributorName, | ||
| hasHidden = true, | ||
| }: { | ||
| contributorName?: string; | ||
| hasHidden?: boolean; | ||
| }): Promise<{ items: ContributorEntity[] }> { | ||
| const query = this.contributorModel | ||
| .query() | ||
| .select("contributors.*") | ||
| .select( | ||
| raw( | ||
| "COALESCE(ARRAY_AGG(DISTINCT jsonb_build_object('id', projects.id, 'name', projects.name)) FILTER (WHERE projects.id IS NOT NULL), '{}') AS projects", | ||
| ), | ||
| ) | ||
| .leftJoin("git_emails", "contributors.id", "git_emails.contributor_id") | ||
| .leftJoin("activity_logs", "git_emails.id", "activity_logs.git_email_id") | ||
| .leftJoin("projects", "activity_logs.project_id", "projects.id") | ||
| .groupBy("contributors.id") | ||
| .withGraphFetched("gitEmails"); | ||
|
|
||
| if (!hasHidden) { | ||
| query.whereNull("contributors.hiddenAt"); | ||
| case ContributorOrderBy.LAST_ACTIVITY_DATE: { | ||
| query.orderBy( | ||
| ContributorOrderBy.LAST_ACTIVITY_DATE, | ||
| SortType.DESCENDING, | ||
| ); | ||
| break; | ||
| } |
There was a problem hiding this comment.
Looks like we don't need all this logic and we can do it this way:
query.orderBy(orderBy, SortType.DESCENDING);
in case we need another sortType, this should be passed to query parameters. And for query params validation itself we need to have a validation schema which will restrict orderBy only to two options
| @@ -0,0 +1,6 @@ | |||
| const ContributorOrderBy = { | |||
There was a problem hiding this comment.
| const ContributorOrderBy = { | |
| const ContributorOrderByKey = { |
GvoFor
requested review from
liza-veis,
Vitaliistf,
perekotypole and
kolibri753
...ed/src/modules/contributors/libs/validation-schemas/contributor-get-all.validation-schema.ts
Show resolved
Hide resolved
|
I'm sure I miss some buisness logic bugs while resolving conflicts, so I'll try to find them out tomorrow |
| return results.map(({ projects, ...projectGroup }) => { | ||
| const [project] = projects; | ||
| return results | ||
| .filter(({ projects }) => projects.length) |
There was a problem hiding this comment.
This check for cases when user deletes project, which has some project groups. After such a deletion in the DB project groups are remained in project_groups table, but they relation with projects are lost (corresponding record in projects_to_project_groups are deleted). So without this .filter, project in const [project] = projects; expression is undefined which leads to Reading property 'id' of undefined error.
It would be better to remove this filter after we fix mentioned issue.
There was a problem hiding this comment.
we need to remove this. This shouldn't be the case for project groups. We need to create a migration to make the project field required and delete groups by cascade
| CREATED_AT: "created_at", | ||
| LAST_ACTIVITY_DATE: "last_activity_date", |
There was a problem hiding this comment.
So as we agreed, maybe we should use camelCase here? (I think that this might be fixed in ticket regarding quality criteria, but if you'll fix anything else, you might to fix this also).
There was a problem hiding this comment.
@Vitaliistf can we fix this in your PR please? we need this pr to be merged to fix deployment
Features
Added project select above contributors table
Screenshots