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.

Sign up for GitHub

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

Jump to bottom

Deadline notification #472

Closed
wants to merge 12 commits into from
Closed

Deadline notification #472

wants to merge 12 commits into from

Conversation

yudukikun5120
Copy link
Collaborator

This is the first PR for this feature. This PR includes:

  • The assignment manager. which fetches assignment information when users log in and keeps its consistency thanks to the PK constraint of IndexedDB.
  • Chrome notifications. which are invoked when the assignment deadline is approaching. The UI is shown below.
スクリーンショット 2022-11-10 18 49 37
  • Opt-in. Users can turn off this feature themselves and this feature is turned off by default.

This PR does NOT include:

  • Specification of the time before the deadline. The default value is 1 day.

@mkobayashime I would like to ask for your code review.

@yudukikun5120 yudukikun5120 added the enhancement New feature or request label Nov 10, 2022
@yudukikun5120 yudukikun5120 self-assigned this Nov 10, 2022
@yudukikun5120 yudukikun5120 linked an issue Nov 10, 2022 that may be closed by this pull request
Assignment deadlines notification #451
Open
@mkobayashime
Copy link
Member

@yudukikun5120
Could you rebase to the latest main to get rid of off-topic commits?

@yudukikun5120
Copy link
Collaborator Author

@mkobayashime I rebased the code. Could I ask you for a code review?

@codeclimate
Copy link

codeclimate bot commented Nov 11, 2022

Code Climate has analyzed commit ed0bc6d and detected 0 issues on this pull request.

View more on Code Climate.

@mkobayashime
Copy link
Member

@yudukikun5120
I'm gonna concentrate on reviewing and refactoring (to the standards of the existing codebase) what was merged in #450 and release it firstly, so I kindly ask you to wait for a while for the review in this PR.

@yudukikun5120 yudukikun5120 marked this pull request as draft November 13, 2022 12:42
@yudukikun5120
Copy link
Collaborator Author

@mkobayashime I found more work is needed and converted this PR to a draft. I appeal to you for waiting for a little bit.

codeclimate[bot]
codeclimate bot reviewed Dec 4, 2022
View reviewed changes
src/methods/AssignmentSync.ts
deadline: this.getAssignmentDeadline(row),
})

private syncAssignmentData = (
Copy link

Choose a reason for hiding this comment

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

Similar blocks of code found in 2 locations. Consider refactoring.

src/methods/AssignmentSync.ts
)
})

public putAssignmentData = (
Copy link

Choose a reason for hiding this comment

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

Similar blocks of code found in 2 locations. Consider refactoring.

@codeclimate
Copy link

codeclimate bot commented Dec 4, 2022

Code Climate has analyzed commit b4a0d85 and detected 2 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 2

View more on Code Climate.

@yudukikun5120
Copy link
Collaborator Author

@mkobayashime
I close this PR because this feature is too hard to debug if it's completed. But I have another plan and I'll work on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@codeclimate CodeClimate codeclimate left review comments

@mkobayashime mkobayashime Awaiting requested review from mkobayashime mkobayashime is a code owner

Assignees

@yudukikun5120 yudukikun5120

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Assignment deadlines notification
2 participants
@yudukikun5120 @mkobayashime