10000 Work always on one idSite in API.getReportMetadata · Issue #7834 · matomo-org/matomo · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Work always on one idSite in API.getReportMetadata #7834

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.

Sign up for GitHub

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

Jump to bottom
Closed
tsteur opened this issue May 5, 2015 · 8 comments
Closed

Work always on one idSite in API.getReportMetadata #7834

tsteur opened this issue May 5, 2015 · 8 comments
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Milestone
3.0.0-b1

Comments

@tsteur
Copy link
Member
tsteur commented May 5, 2015

There are currently two API methods that operate on multiple idSites:

  • API.getSegmentsMetadata (idSites = 'Array')
  • API.getReportMetadata (idSites = '', period = '', ...)

I'm not quite sure why this is the case. Handling multiple idSites becomes complicated when dealing with #7131 and #4734 . In those issues we want plugin developers to be able to define a new type. A "type" could be a plugin that defines a type "Website" or "MobileApp". Different types need different reports and different naming of reports.

How would we behave eg when someone requests the report metadata for many sites, that all have a different type. Re renaming we'd have to fall back to generic wording, this would be easy. But would we only return reports that are supported by both types, or all reports that are supported by any report?

I'm not sure what the use case was behind this but I think we should remove it to keep things easy. This will break BC. So we should do it for Piwik 3.0.0
@tsteur tsteur added Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change. c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. labels May 5, 2015
@tsteur tsteur added this to the Piwik 3.0.0 milestone May 5, 2015
@mattab
Copy link
Member
mattab commented May 6, 2015

I'm not sure what the use case was behind this but I think we should remove it to keep things easy. This will break BC. So we should do it for Piwik 3.0.0

I'm pretty sure it was done 4+ years ago for performance reasons. Today these kind of solutions don't make sense

definitely 👍 to remove idSites and consistently use idSite.

@tsteur tsteur mentioned this issue May 7, 2015
Open
13 tasks
@mattab mattab changed the title Work always on one idSite in API methods Work always on one idSite in API.getSegmentsMetadata and API.getReportMetadata Aug 13, 2015
@tsteur tsteur mentioned this issue Nov 2, 2015
Closed
@mattab
Copy link
Member
mattab commented Nov 2, 2015

Maybe we could do this in 2.15.1 since we need it for #9129

Suggestion

  • add the new parameter idSite
  • Keep BC when idSites=N:
    • leave the idSites parameter for BC, so that API calls in using one website ID will still work.
    • if API is called with multiple websites, return frienly exception message

@mattab
Copy link
Member
mattab commented Nov 2, 2015

Note: ideally we shouldn't break any API during minor release but it's acceptable IMO in this case because we only partially break the API (only in use case when called with more than one website), which impacts very few users (rough guess: almost nobody) who would use getSegmentsMetadata with multiple site IDs.

@tsteur
Copy link
Member Author
tsteur commented Nov 2, 2015

A partial break is pretty much never acceptable in a patch release unless it is maybe security related or so. We don't have to do it in 2.15.1 as it is probably most likely only used with one idSite anyway. When there are multiple idSites given we will simply not return a segment from custom dimensions as the sites don't have any segments in common. Or maybe we will return some segments for custom dimensions we will see. The custom dimensions plugin will be on the marketplace first anyway so no need to break anything for this

@mattab mattab modified the milestones: 3.0.0-b1, 3.0.0 Feb 8, 2016
@tsteur
Copy link
Member Author
tsteur commented Aug 8, 2016 •
edited by mattab
Loading

Looks like it will be quite hard to do this for API.getSegmentsMetadata. I started changing behaviour to only one site but problem is this method is used by Piwik\Segment https://github.com/piwik/piwik/blob/2.16.1/core/Segment.php#L117 with multiple idsites which is used eg by Archive::build() with multiple sites. To solve this I would also almost need to change archiver etc. I'd say for API.getSegmentsMetadata we rather postpone it to Piwik 4 or when we refactor the archiver etc in #7470 .

I am changing behaviour for API.getReportMetadata but need to see what tests are saying :)

@tsteur tsteur mentioned this issue Aug 8, 2016
Merged
@mattab
Copy link
Member
mattab commented Aug 16, 2016

Do you want to move this issue back to 3.0.0 milestone? (most of these issues will be moved to 4.0.0 milestone)

@mattab mattab changed the title Work always on one idSite in API.getSegmentsMetadata and API.getReportMetadata Work always on one idSite in API.getReportMetadata Aug 16, 2016
@mattab mattab mentioned this issue Aug 16, 2016
Closed
@mattab
Copy link
Member
mattab commented Aug 16, 2016

just realised you fixed most of this issue already, so renamed this issue + created follow up issue: #10403

@tsteur
Copy link
Member Author
tsteur commented Sep 13, 2016

This is resolved and new issue was created for #10403

@tsteur tsteur closed this as completed Sep 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
c: Platform For Matomo platform changes that aren't impacting any of our APIs but improve the core itself. Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change.
Milestone
3.0.0-b1
Development

No branches or pull requests
2 participants
@tsteur @mattab
0