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

Update alert shouldn't call update card API #36395

Closed
qnkhuat opened this issue Dec 5, 2023 · 1 comment · Fixed by #36866
Closed

Update alert shouldn't call update card API #36395

qnkhuat opened this issue Dec 5, 2023 · 1 comment · Fixed by #36866
Assignees
@rafpaf
Labels
.Frontend good first issue A starter issue that is good for someone new to the codebase or is a new contributor Misc/API Reporting/Alerts

Comments

@qnkhuat
Copy link
Contributor

qnkhuat commented Dec 5, 2023 •
edited

Describe the bug

When updating an alert, we call PUT /api/card/:id before calling PUT /api/alert/:id even though the card didn't change

To Reproduce

  1. Setup email
  2. Create a new card
  3. Setup alert and save

Expected behavior

We probably don't need the update card API call

Logs

No response

Information about your Metabase installation

master

Severity

low, but it slows down the application and burn uncessary CPU cycles

Additional context

No response
@qnkhuat qnkhuat added Misc/API Reporting/Alerts .Frontend good first issue A starter issue that is good for someone new to the codebase or is a new contributor labels Dec 5, 2023
@rafpaf rafpaf self-assigned this Dec 14, 2023
@rafpaf
Copy link
Contributor

rafpaf commented Dec 14, 2023 •
edited

Hi @qnkhuat, I'm not yet sure how to reproduce this. When I make an alert, I only see a POST to /api/alert/. Here's a short screencast.

Never mind, I can reproduce this bug now!

rafpaf added a commit that referenced this issue Dec 15, 2023
@rafpaf
When updating an alert, don't also update the question
5708acf 
The additional call to apiUpdateQuestion is not needed, since you can't
edit an alert without first saving the question

Fixes #36395
@rafpaf rafpaf mentioned this issue Dec 15, 2023
Merged
1 task
rafpaf added a commit that referenced this issue Dec 19, 2023
@rafpaf
When updating an alert, don't also update the question
5d58e33 
The additional call to apiUpdateQuestion is not needed, since you can't
edit an alert without first saving the question

Fixes #36395
rafpaf added a commit that referenced this issue Dec 20, 2023
@rafpaf
When updating an alert, don't also update the question
de7b110 
The additional call to apiUpdateQuestion is not needed, since you can't
edit an alert without first saving the question

Fixes #36395
rafpaf added a commit that referenced this issue Dec 21, 2023
@rafpaf
When updating an alert, don't also update the question
aef580b 
The additional call to apiUpdateQuestion is not needed, since you can't
edit an alert without first saving the question

Fixes #36395
@rafpaf rafpaf mentioned this issue Jan 3, 2024
Closed
rafpaf added a commit that referenced this issue Jan 3, 2024
@rafpaf
When updating an alert, don't also update the question
ba3b966 
The additional call to apiUpdateQuestion is not needed, since you can't
edit an alert without first saving the question

Fixes #36395
rafpaf added a commit that referenced this issue Jan 3, 2024
@rafpaf
When updating an alert, don't also update the question (#36866)
9c2fed9 
The additional call to apiUpdateQuestion is not needed, since you can't edit an alert without first saving the question

Fixes #36395
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rafpaf rafpaf

Labels
.Frontend good first issue A starter issue that is good for someone new to the codebase or is a new contributor Misc/API Reporting/Alerts
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

When updating an alert, don't also update the question metabase/metabase
2 participants
@rafpaf @qnkhuat