Skip to content

Conversation

@AltamashShaikh
Copy link
Contributor

Description

Refactored CustomAlerts UI to support selection of medium to send alert

Issue No

#PG-4530

Steps to Replicate the Issue

  1. View Custom Alerts
  2. You can see no option to select medium, email and mobile is always visible
  3. Now checkout this PR
  4. You will see a new dropdown to select medium
  5. Also it will now throw error if, medium selected, but no valid parameter set, example report_mediums: ["email"] and emailME and additionalEmails parameter is empty.

Checklist

  • [✔] Tested locally or on demo2/demo3?
  • [✔] New test case added/updated?
  • [✔] Are all newly added texts included via translation?
  • [✔] Are text sanitized properly? (Eg use of v-text v/s v-html for vue)
  • [✔] Version bumped?

@AltamashShaikh
Copy link
Contributor Author

@james-hill-matomo @snake14 I have to create a new PR from this branch to add Slack as an option.

Rough PR on local adds this
image

Copy link

@james-hill-matomo james-hill-matomo left a comment

Choose a reason for hiding this comment

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

Error messages not 100% consistent - e.g. "Report mediums must not be empty" shows when the UI box is labelled "Send via"

Quite a few bad UX things going on, but out of scope of this ticket.

I wasn't able to test what happens if you add a phone number and then choose "Send via" email only - is that supported OK?

<rule ref="Generic.Files.LineLength">
<properties>
<property name="lineLimit" value="250" />
<property name="lineLimit" value="260" />

Choose a reason for hiding this comment

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

Naughty? 😂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yaa it was giving an error of 255 characters, so increased it to 260

@AltamashShaikh
Copy link
Contributor Author

Error messages not 100% consistent - e.g. "Report mediums must not be empty" shows when the UI box is labelled "Send via"

Quite a few bad UX things going on, but out of scope of this ticket.

I wasn't able to test what happens if you add a phone number and then choose "Send via" email only - is that supported OK?

It would be set to empty if the medium is not set - https://github.com/matomo-org/plugin-CustomAlerts/pull/208/files#diff-289a75deac5516af6414cbbad6082c750fb8b192830f6b8fdf4348a97091716dR188

@AltamashShaikh AltamashShaikh merged commit fe2140a into 5.x-dev Sep 18, 2025
23 of 32 checks passed
@AltamashShaikh AltamashShaikh deleted the PG-4530-refactor-UI branch September 18, 2025 05:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants