Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not send idGoal=0 parameter for goals overview since 0 is for orders #17554

Closed
wants to merge 1 commit into from

Conversation

diosmosis
Copy link
Member

Description:

Fixes #17502

In the past the UI controller method that outputted the goals overview used idGoal=0 to signify it was handling the overview. It would not pass that parameter on to the API, since the API treats idGoal=0 as IDGOAL_ORDER.

This means, currently, we are displaying order goal information in place of the goals overview for scheduled reports. To fix this, we can just not send the idGoal to the API.

The tests are failing, most seem like they are expected. The TSV test however changes the name of the report in the output (though this doesn't happen locally for me).

cc @tsteur for prioritization

Review

  • Functional review done
  • Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
  • Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • Security review done see checklist
  • Code review done
  • Tests were added if useful/possible
  • Reviewed for breaking changes
  • Developer changelog updated if needed
  • Documentation added if needed
  • Existing documentation updated if needed

@diosmosis diosmosis added this to the 4.4.0 milestone May 25, 2021
@diosmosis diosmosis added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label May 25, 2021
@diosmosis diosmosis marked this pull request as draft May 25, 2021 23:07
@mattab mattab modified the milestones: 4.4.0, 4.3.0 May 26, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Jun 9, 2021

If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.

@github-actions github-actions bot added the Stale The label used by the Close Stale Issues action label Jun 9, 2021
@diosmosis diosmosis added the Do not close PRs with this label won't be marked as stale by the Close Stale Issues action label Jun 9, 2021
@github-actions github-actions bot removed the Stale The label used by the Close Stale Issues action label Jun 10, 2021
@tsteur
Copy link
Member

tsteur commented Jul 27, 2021

Will close this one as it's not high priority and issue is not scheduled.

@tsteur tsteur closed this Jul 27, 2021
@sgiehl sgiehl deleted the 17502-fix-goals-overview-api branch April 5, 2023 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Do not close PRs with this label won't be marked as stale by the Close Stale Issues action not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Email report: Goals Overview is empty
3 participants