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

fix: 4th attempt for: now able to delete schedule even if active #435

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Wizardbacon13
Copy link
Contributor

@Wizardbacon13 Wizardbacon13 commented Nov 17, 2024

Fixes #401
First 2 images was after changes, third image is before changes

Also, SCSS seems to have an update where they no longer support @import and now use @use, which is why I changed certain .scss files or else "pnpm build" wouldn't have worked due to errors

Also also, I changed Instructor.ts because it outputs an error due to it not seeing any data and thinking there should be an error, making me fail one of the github checks, so I modified it so it doesn't output this error when there're no schedules.

1
2
3


This change is Reviewable

Copy link

sentry-io bot commented Nov 17, 2024

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: src/pages/background/lib/deleteSchedule.ts

Function Unhandled Issue
deleteSchedule Error: Cannot delete active schedule deleteSchedu...
Event Count: 3 Affected Users: 2
deleteSchedule Error: Schedule h4HkkyYCWZ45 does not exist delet...
Event Count: 1 Affected Users: 20

Did you find this useful? React with a 👍 or 👎

@DereC4 DereC4 requested review from Razboy20 and doprz November 17, 2024 12:24
@DereC4 DereC4 added this to the v2.1.0 milestone Nov 17, 2024
@DereC4 DereC4 added the bugfix label Nov 17, 2024
@IsaDavRod IsaDavRod self-requested a review November 17, 2024 14:08
Copy link
Member

@IsaDavRod IsaDavRod left a comment

Choose a reason for hiding this comment

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

LGTM (Sam should address Instructor.ts through his PR)

Copy link
Collaborator

Choose a reason for hiding this comment

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

This file shouldn't be updated in this PR. The Chromatic bug is resolved with #425

@doprz
Copy link
Collaborator

doprz commented Nov 20, 2024

Will review with more detail soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Delete schedule even if active
4 participants