Skip to content

Conversation

@tuantran0910
Copy link
Contributor

Closes #4271

📑 Description

When provisioning a provider and sending an alert with an incorrect providerId (one that does not belong to the provisioned provider), the alert gets permanently associated with the wrong provider.

If the same alert is sent again (with partial deduplication) using the correct providerId, it still retains the previously assigned incorrect providerId. We should allow the providerId to be updated to the latest value.

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

ℹ Additional Information

Here’s a clearer and more polished version of your text:

I have configured alerts to be sent to Keep via a generic webhook endpoint. For the provisioned provider A with ID 123, I decided to add the query parameter ?provider_id=123.

On the Provider page, each provider card was incorrectly displaying the id field instead of the name field. I have updated it to display the name field correctly.

Before

image

After

image

Additionally, the spacing between the icon and the provider name was too tight, affecting readability. I have added some spacing for better clarity.

Before

image

After

image

@vercel
Copy link

vercel bot commented Mar 28, 2025

@tuantran0910 is attempting to deploy a commit to the KeepHQ Team on Vercel.

A member of the Team first needs to authorize it.

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. Provider Providers related issues UI User interface related issues labels Mar 28, 2025
@tuantran0910 tuantran0910 changed the title Allow using latest providerId and alter UI display feat: allow using latest providerId and alter UI display Mar 28, 2025
@talboren
Copy link
Member

@tuantran0910 thank you for the contribution. I have to admit that I'm missing the before/after difference. Can you point me to where can I see the changes? It seems like both are still displaying the id of the provider.

@vercel
Copy link

vercel bot commented Mar 28, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
keep ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 30, 2025 7:00am

@talboren talboren self-requested a review March 28, 2025 17:20
@tuantran0910
Copy link
Contributor Author

tuantran0910 commented Mar 28, 2025

Hi @talboren, here is the difference, just rename Id to Name for clarity.

image

Actually, these are the names of the providers and their IDs are UUID-like string like this :D

image

Copy link
Member

@talboren talboren left a comment

Choose a reason for hiding this comment

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

LGTM

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 28, 2025
@tuantran0910
Copy link
Contributor Author

Thanks @talboren for the approval :D Can you help me to see what happened with the E2E tests. It seems that I don't have permission for those tests right ?

@talboren
Copy link
Member

Thanks @talboren for the approval :D Can you help me to see what happened with the E2E tests. It seems that I don't have permission for those tests right ?

I think it has something to do with a change in the e2e workflow we did, which caches and pulls images, and requires some other permissions now. I've re-run it and if it fails again, will fix and merge on Sunday.

@talboren talboren merged commit 79775ca into keephq:main Mar 30, 2025
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer Provider Providers related issues size:S This PR changes 10-29 lines, ignoring generated files. UI User interface related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🐛 Bug]: ProviderId should be updated to the newest value

2 participants