Skip to content

Conversation

boomboomxx
Copy link

@boomboomxx boomboomxx commented Dec 21, 2024

⚠️⚠️⚠️ Since we do not accept all types of pull requests and do not want to waste your time. Please be sure that you have read pull request rules:
https://github.com/louislam/uptime-kuma/blob/master/CONTRIBUTING.md#can-i-create-a-pull-request-for-uptime-kuma

Tick the checkbox if you understand [x]:

  • [√] I have read and understand the pull request rules.

Description

Sometimes we need to notify specified users in a webhook instead of all users in Dingtalk, and right now it only supports @everyone.
According to Doc, we can tell that it supports that feature.
Therefore, this PR adds the feature and adds it to the test message. It supports two ways of specifying:

  1. mobile list
  2. user ID list

Type of change

Please delete any options that are not relevant.

  • User interface (UI)
  • Other

Checklist

  • [√] My code follows the style guidelines of this project
  • [√] I ran ESLint and other linters for modified files
  • [√] I have performed a self-review of my own code and tested it
  • [√] I have commented my code, particularly in hard-to-understand areas (including JSDoc for methods)
  • [√] My changes generates no new warnings
  • [√] My code needed automated testing. I have added them (this is optional task)

Screenshots (if any)

image
image

Copy link
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.

Overall, looks like a reasonable change.
I have left a few changes that I would like to see

@CommanderStorm CommanderStorm added the pr:please address review comments this PR needs a bit more work to be mergable label Dec 22, 2024
@boomboomxx
Copy link
Author

boomboomxx commented Dec 23, 2024

Overall, looks like a reasonable change. I have left a few changes that I would like to see

I have a question, how do I check the required rules in vue-multiselect.
Do you have any ideas?

@CommanderStorm
Copy link
Collaborator

required rules?

@boomboomxx
Copy link
Author

required rules?

Like <input required>, the vue-multiselect dose not have required attr.

@boomboomxx
Copy link
Author

boomboomxx commented Dec 24, 2024

Hi @CommanderStorm, Please help to review again.
I changed the input to vue-multiselect and changed the data validation rules to do the data is empty compatibility.
Currently there is only one problem, how to check if the value of vue-multiselect is null on submit time

@CommanderStorm CommanderStorm changed the base branch from master to 2.1.X January 24, 2025 19:13
@CommanderStorm CommanderStorm added this to the 2.1.0 milestone Jan 24, 2025
Copy link
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.

Thanks, LGTM now

@CommanderStorm CommanderStorm added pr:depends on other pending other things to be done first and removed pr:please address review comments this PR needs a bit more work to be mergable labels Jan 24, 2025
@CommanderStorm CommanderStorm changed the title Add @People feature in DingTalk notifications feat: allow users to @People in DingTalk notifications Jan 24, 2025
@louislam louislam merged commit 9579df3 into louislam:2.1.X Mar 3, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:depends on other pending other things to be done first

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants