Page MenuHomePhabricator

Implement validation of event goals (behaviour layer)
Closed, ResolvedPublic

Description

Acceptance Criteria:

  • Add event goals as a parameter to EventFactory and implement validation for them
  • Given that an organizer does not select one of the goal metrics from the dropdown, but they add a goal number, and then they try to save, they should see the following message at the top:
    • "You must add a goal metric and target number in order to save an event goal."
  • Given that an organizer selects a goal types from the dropdown, but they do not add a goal number, and then they try to save, they should see the following message at the top:
    • "You must add a goal metric and target number in order to save an event goal."
  • Given API users send an invalid goal metric, Then and error message with the text below will be returned (This would happen only for API users)
    • "You must choose a valid goal metric."
  • Given API users send an invalid goal target (not a positive integer or null), Then and error message with the text below will be returned (This would happen only for API users)
    • "Goal target number must be a positive number."
  • Given users send a goal metric and target for an event where Contribution statistics is not enabled, then an error message with the text below will be returned:
    • "You must turn on Contribution statistics in order to add an event goal."

Event Timeline

Daimona renamed this task from Behaviour layer, goal validation (EventFactory) to Implement validation of event goals (behaviour layer).Jan 13 2026, 10:42 PM
Daimona updated the task description. (Show Details)

thanks ilana! i wonder if the messages should:

  1. use the word 'choose'/'set' instead of 'pick', the latest sounds a little bit more informal and 'choose'/'set' are the verbs that we use in the fields.
  2. is it necessary to say 'to save a goal'? the user already knows that they're trying to save, maybe it's a little bit redundant?

what about: "Choose a goal type" and "Set a target number"? if too vague, I think we can say 'Choose a goal type before saving' and 'Set a target number before saving'.

Open questions:

  • What is the minimum number and maximum number (if we want) for goals?
  • Should we hide the number field if no goal type selected?
ifried updated the task description. (Show Details)
ifried updated the task description. (Show Details)

Decisions reached, after discussion between me & @JFernandez-WMF:

  • If user tries to save with incomplete goal information, they should see the following text: "This value is required to save a goal."
  • Minimum number for goal: 1
  • Maximum number for goals: No maximum for now, but we can re-assess later if this causes issues
  • Do not hide the number field if no goal type selected, so that the user understands everything upfront regarding what is required
  • Do not hide the number field if no goal type selected, so that the user understands everything upfront regarding what is required

Argh, apologies, this is my fault for suggesting to *hide* it, but really I should've said *disable*, not hide. Anyway, we can proceed as decided above.

@ifried @JFernandez-WMF We have discussed this a bit in the engineering meeting. The way the registration form currently works is that all error messages are shown in a summary at the top of the form, regardless of the field where they originate. There is already a task about changing this behaviour, but it has not been worked on: T305690. That is not entirely straightforward (a few refactorings involved, and potentially error message changes), so if we wanted to change this, we would need to set some time aside explicitly for that task, and now is probably not a good time to do it.

The reason why we discussed this and I'm bringing it up, is that the error messages use "This value"; however, if the message appears at the top of the form, it is not clear what is the "this value" that it refers to; so, the messages should include more information to locate the problem.

Discussion from March 2 2026 meeting:

  • @JFernandez-WMF and I discussed this in the meeting today with the team. We have updated the AC to reflect having one message for both cases in which one of the values is not complete, which will be displayed at the top.

Change #1247592 had a related patch set uploaded (by Cmelo; author: Cmelo):

[mediawiki/extensions/CampaignEvents@master] Implement validation of event goals

https://gerrit.wikimedia.org/r/1247592

Change #1247592 merged by jenkins-bot:

[mediawiki/extensions/CampaignEvents@master] Implement validation of event goals

https://gerrit.wikimedia.org/r/1247592

Tested the API part of it:

  • Add event goals as a parameter to EventFactory and implement validation for them

โžก๏ธ Tested indirectly.

  • Given that an organizer does not select one of the goal metrics from the dropdown, but they add a goal number, and then they try to save, they should see the following message at the top:
    • "You must add a goal metric and target number in order to save an event goal."

โœ… With error key campaignevents-error-goal-type-required.

  • Given that an organizer selects a goal types from the dropdown, but they do not add a goal number, and then they try to save, they should see the following message at the top:
    • "You must add a goal metric and target number in order to save an event goal."

โœ… With error key campaignevents-error-goal-target-required.

  • Given API users send an invalid goal metric, Then and error message with the text below will be returned (This would happen only for API users)
    • "You must choose a valid goal metric."

โœ…โš ๏ธ For API users, the error is a normal validation failure, e.g. "Invalid request body: Unrecognized value for parameter \"goal_type\": garbage.". That's fine though, as it is specific to the API, and independent of the general-purpose message.

  • Given API users send an invalid goal target (not a positive integer or null), Then and error message with the text below will be returned (This would happen only for API users)
    • "Goal target number must be a positive number."

โœ…โš ๏ธ Like above, for API users this is a normal validation failure, e.g. Invalid request body: The value \"0\" for parameter \"goal_target\" must be no less than 1.. No problem though.

  • Given users send a goal metric and target for an event where Contribution statistics is not enabled, then an error message with the text below will be returned:
    • "You must turn on Contribution statistics in order to add an event goal."

โœ… With error key campaignevents-error-goal-requires-contribution-tracking