Skip to content

Fix do nothing erroneous api call for Pagerduty#6231

Merged
CommanderStorm merged 5 commits intolouislam:masterfrom
maldotcom2:bugfix/pagerduty-do-nothing-6105
Oct 27, 2025
Merged

Fix do nothing erroneous api call for Pagerduty#6231
CommanderStorm merged 5 commits intolouislam:masterfrom
maldotcom2:bugfix/pagerduty-do-nothing-6105

Conversation

@maldotcom2
Copy link
Copy Markdown
Contributor

❗ Important Announcements

Click here for more details:

⚠️ Please Note: We do not accept all types of pull requests, and we want to ensure we don’t waste your time. Before submitting, make sure you have read our pull request guidelines: Pull Request Rules

🚫 Please Avoid Unnecessary Pinging of Maintainers

We kindly ask you to refrain from pinging maintainers unless absolutely necessary. Pings are for critical/urgent pull requests that require immediate attention.

📋 Overview

This change fixes a bug where the the do-nothing option for "Auto resolve or acknowledged" still results in an API call to PagerDuty, and this call contains an invalid parameter "0" for "event_action". The issue occurs because the current settings value of "0" for do nothing, is a string of "0" and therefore is truthy when it is evaluated by the backend logic. This results in the early return being missed and the API call is made carrying the invalid value of "0".

By storing this settings value as an empty string, it is evaluated as falsy and becomes null which satisfies the early return and no API call is made, which is the desired behaviour of "no nothing".

Changing the way this value is stored in the database is low impact and means the backend logic can remain as it is, instead of having to explicitly check for "0".

For users who have selected "do nothing" before this change is applied, the UI drop-down may be nulled, however this is cosmetic only and the existing setting of "0" remains in the database until the user re-selects "do nothing" at which point the change takes place. Until the user does this, the existing behaviour persists. Users who had selected resolve or acknowledge, are not impacted at all.

🛠️ Type of change

  • 🐛 Bugfix (a non-breaking change that resolves an issue)
  • ✨ New feature (a non-breaking change that adds new functionality)
  • ⚠️ Breaking change (a fix or feature that alters existing functionality in a way that could cause issues)
  • 🎨 User Interface (UI) updates
  • 📄 New Documentation (addition of new documentation)
  • 📄 Documentation Update (modification of existing documentation)
  • 📄 Documentation Update Required (the change requires updates to related documentation)
  • 🔧 Other (please specify):
    • Provide additional details here.

📄 Checklist

  • 🔍 My code adheres to the style guidelines of this project.
  • 🦿 I have indicated where (if any) I used an LLM for the contributions
  • ✅ I ran ESLint and other code linters for modified files.
  • 🛠️ I have reviewed and tested my code.
  • 📝 I have commented my code, especially in hard-to-understand areas (e.g., using JSDoc for methods).
  • ⚠️ My changes generate no new warnings.
  • 🤖 My code needed automated testing. I have added them (this is an optional task).
  • 📄 Documentation updates are included (if applicable).
  • 🔒 I have considered potential security impacts and mitigated risks.
  • 🧰 Dependency updates are listed and explained.
  • 📚 I have read and understood the Pull Request guidelines.

Copy link
Copy Markdown
Collaborator

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

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

This will only fix the issue for newly saved notifications.
Could you adjust the if-statement in the backend instead?

@maldotcom2
Copy link
Copy Markdown
Contributor Author

Change made to suit preference for backend logic fix. The style of the changes align with other similar provider backend logic, such as Splunk. Tested on Pagerduty, all settings behave as intended.

Copy link
Copy Markdown
Collaborator

@CommanderStorm CommanderStorm left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the fix

@CommanderStorm CommanderStorm added this to the 2.1.0 milestone Oct 27, 2025
@CommanderStorm CommanderStorm merged commit 38ec3bc into louislam:master Oct 27, 2025
19 checks passed
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.

"Do nothing" option for the Pagerduty notification making API call which fails

2 participants