-
Notifications
You must be signed in to change notification settings - Fork 79
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
Merge Feature bug trimming branch to main #1331
base: master
Are you sure you want to change the base?
Conversation
… standards (#1311) According to GitHub standards, the value for closed state when filtering for issues should be `closed` instead of `close` Let's fix the value to the correct value: `closed`, and add the valid `all` value together
This new implementation is needed to support the new phase of CATcher. Multiple phases have to be updated to support the phase. These files are phase.model.ts, session.model.ts, permission.service.ts and issue.service.ts. The new implementation of the phase is similar to how previous phases are implemented. The new public_data setting for the new phase can be found in PR #1308.
As we are introducing a new bug-trimming phase for students to trim their bugs after the bug-reporting phase, having a single table of issues posted is not sufficient. Let's implement a new UI where users can view 2 separate tables (one for open and the other for closed issues), while allowing users to restore closed issues from the closed issues table too.
Issue responses from the REST API are being accepted as the GithubIssue class. This is incorrectly typed since the REST API returns a lower cased state, while GithubIssue expects an upper-cased state. This causes an inconsistency when updating the issues in issue.service.ts with the response from these REST API queries, which is done during close and reopen. This prevents us from filtering based on the state correctly. Let's add a new type called GithubRestIssue that accurately represents the issue response from the REST API. We then add a factory method in GithubIssue that creates a GithubIssue from a GithubRestIssue and replace all incorrect GithubIssue usages with GithubRestIssue.
The new Bug Trimming phase allows users to edit labels of their bugs, but not the title or description. Let's create a new issue component under the bug trimming module where users can view their bugs and edit the labels.
Make it easier to see which issues are deleted Allow for further differentiation between issues that the user wants and doesn't want by striking-through all text in the table. This helps to give a visual difference between the un-deleted issues, which should prevent confusion when the table headers are not visible. Styles are applied via CSS override when referencing the component in CSS, using ::ng-deep.
Fix strikethrough on Safari
Remove feature branch from github-actions.yml This feature branch was used to create the bug-trimming phase and it required to run the tests when we merge into it
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1331 +/- ##
==========================================
- Coverage 58.39% 58.26% -0.13%
==========================================
Files 104 104
Lines 2579 2600 +21
Branches 291 295 +4
==========================================
+ Hits 1506 1515 +9
- Misses 1022 1033 +11
- Partials 51 52 +1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a small removal of a comment to prevent further confusion. LGTM otherwise
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Tested locally and works fine!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for all your efforts. Tested and working, LGTM.
We should wait for more people to validate its working state before merging since this is a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
LGTM! No issues with testing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issues deleted during bug reporting phase can still be restored during bug trimming phase. Is this intended?
It is intentional |
Ah nvm my local repo did not have the latest commits 🤡 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Adding an additional note to update catcher public data repo with the bug trimming phase once this PR is merged. |
Summary:
Fixes #1301
Changes Made:
phaseBugTrimming
Proposed Commit Message: