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: fixes an issue with remove-signup where multiple rows were modified #284

Merged
merged 1 commit into from
May 19, 2024

Conversation

ssilve1989
Copy link
Owner

@ssilve1989 ssilve1989 commented May 19, 2024

There was a range error on the sheet modification for clear party removals that was overlooked.
In addition the /remove-signup command has been enhanced to do the following

  • Remove a signup that matches no DB entry. - Should return error saying this only works for signups made with the bot, not prior to the bot because we can't verify if they're trying to remove their own or someone elses signup
  • Remove signup that matches DB entry but has not yet been approved. - This should remove their entry from the DB but will not check the google sheet since we don't expect to find a row there.
  • Remove signup that matches DB entry and has been approved. - This should remove their entry from the DB and check the Google Sheet for a matching row. If a matching row exists, it will remove that row from the Google Sheet. If no matching row exists, it will reply saying it removed the signup from the DB but could not find a matching row in the Google Sheet. If they think this is an error they can reach out to a coordinator.

Copy link

codecov bot commented May 19, 2024

Codecov Report

Attention: Patch coverage is 70.51282% with 23 lines in your changes are missing coverage. Please review.

Project coverage is 53.31%. Comparing base (75aafe4) to head (689363f).

Files Patch % Lines
...nds/remove-signup/remove-signup-command.handler.ts 75.00% 14 Missing ⚠️
.../subcommands/remove-signup/remove-signup.consts.ts 61.90% 8 Missing ⚠️
src/sheets/sheets.service.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #284      +/-   ##
==========================================
+ Coverage   53.24%   53.31%   +0.07%     
==========================================
  Files          71       72       +1     
  Lines        3786     3841      +55     
  Branches      194      195       +1     
==========================================
+ Hits         2016     2048      +32     
- Misses       1770     1793      +23     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ssilve1989 ssilve1989 force-pushed the fix/multi-line-remove-clear-sheet branch from dacec92 to 689363f Compare May 19, 2024 13:36
@ssilve1989 ssilve1989 merged commit cd9af41 into master May 19, 2024
8 checks passed
@ssilve1989 ssilve1989 deleted the fix/multi-line-remove-clear-sheet branch May 19, 2024 13:37
ssilve1989 added a commit that referenced this pull request May 19, 2024
🤖 I have created a release *beep* *boop*
---


##
[1.7.1](v1.7.0...v1.7.1)
(2024-05-19)


### Bug Fixes

* fixes an issue with remove-signup where multiple rows were modified
([#284](#284))
([cd9af41](cd9af41))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
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.

1 participant