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

Integrate Grouping Service #313

Merged

Conversation

NereusWB922
Copy link
Contributor

Summary:

Fixes part of #306

Type of change:

  • ✨ New Feature/ Enhancement

Changes Made:

  • Refactor IssueDataTable to use GroupingContextService for data grouping
  • Refactor IssueViewerComponent to use GroupingContextService for grouping
  • Refactor CardViewComponent to render dynamic header for different Group type
  • Refactor HiddenUsersComponent to render different Group type
  • Implement group by selection form in FilterBarComponent

Proposed Commit Message:

Integrate Grouping Service

Implement GroupBy feature to allow users to group the issues/prs
based on different criteria such as milestone, status and etc.

Let's integrate the grouping service in the components.

@NereusWB922 NereusWB922 force-pushed the 306-integrate-grouping-service branch from d912df4 to 930e88e Compare March 24, 2024 05:14
@NereusWB922 NereusWB922 force-pushed the 306-integrate-grouping-service branch from 930e88e to 62a1bbf Compare March 25, 2024 04:33
nknguyenhc
nknguyenhc previously approved these changes Mar 25, 2024
Copy link
Contributor

@nknguyenhc nknguyenhc left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@Arif-Khalid Arif-Khalid left a comment

Choose a reason for hiding this comment

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

An excellent PR which lays the groundwork for switching by group. Perhaps the currGroupBy variable could be stored and updated as a filter since I would consider it a filter variable. This would reduce duplicated code for enhancements regarding filters such as storing filters in the url. I will create an issue for discussion not to be implemented in this PR.

Also a minor complaint about naming, but otherwise a great implementation which no doubt took a lot of work! 😀

src/app/issues-viewer/issues-viewer.component.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@MadLamprey MadLamprey left a comment

Choose a reason for hiding this comment

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

LGTM!

src/app/issues-viewer/issues-viewer.component.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@MadLamprey MadLamprey left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@Arif-Khalid Arif-Khalid left a comment

Choose a reason for hiding this comment

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

LGTM!

@NereusWB922 NereusWB922 merged commit 3420a73 into CATcher-org:main Mar 25, 2024
3 checks passed
@NereusWB922 NereusWB922 deleted the 306-integrate-grouping-service branch March 25, 2024 05:58
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