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.

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

Remove all methods out of plugins/*/API.php that have an @ignore annotation #6535

Closed
tsteur opened this issue Oct 28, 2014 · 4 comments
Closed

Remove all methods out of plugins/*/API.php that have an @ignore annotation #6535

tsteur opened this issue Oct 28, 2014 · 4 comments
Labels
wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it.
Milestone
4.0.0-RC

Comments

@tsteur
Copy link
Member

tsteur commented Oct 28, 2014

Makes no sense by definition. In API.php should be only methods that are callable via the HTTP API.

Instead those methods should be moved in different classes. Such as a Model or something more meaningful.
@tsteur tsteur added the Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change. label Oct 28, 2014
@tsteur tsteur mentioned this issue Oct 28, 2014
Closed
@mattab mattab added this to the Short term milestone Nov 3, 2014
@mattab mattab modified the milestones: Short term, Piwik 3.0.0 Apr 7, 2015
@mattab mattab modified the milestones: 3.0.0-b2, 3.0.0 Oct 1, 2015
@mattab mattab modified the milestones: 3.0.0-b2, 3.0.0-b3 Oct 30, 2016
@mattab mattab modified the milestones: 3.0.0-b3, 3.0.0-b4 Nov 14, 2016
@mattab mattab modified the milestones: 3.0.0-b4, 4.0.0 Dec 1, 2016
@sgiehl
Copy link
Member

sgiehl commented Apr 21, 2020 •
edited

Just to be sure. That would currently include the following API methods:

  • API.getSVGLogoUrl
  • API.hasSVGLogo
  • Annotations.getDateRangeForPeriod
  • CustomVariables.getReservedCustomVariableKeys
  • DBStats.getArchiveTableYear
  • Events.getActionToLoadSubtables
  • Events.getDefaultSecondaryDimension
  • Events.getSecondaryDimensions
  • Goals.getConversions
  • Goals.getNbVisitsConverted
  • Goals.getConversionRate
  • Goals.getRevenue
  • LanguagesManager.getPluginTranslationsForLanguage
  • Live.getLastVisitsForVisitor
  • MobileMessaging.sanitizePhoneNumber
  • MobileMessaging.sendSMS
  • MobileMessaging.getPhoneNumbers
  • MobileMessaging.getActivatedPhoneNumbers
  • MultiSites.getApiMetrics
  • Referrers.getKeywordNotDefinedString
  • Referrers.getCleanKeyword
  • ScheduledReports.getReportMetadata
  • ScheduledReports.allowMultipleReports
  • ScheduledReports.getReportTypes
  • ScheduledReports.getReportFormats
  • ScheduledReports.getReportRecipients
  • ScheduledReports.isSegmentEditorActivated
  • SitesManager.getSitesIdFromTimezones
  • SitesManager.updateSiteCreatedTime
  • UsersManager.initUserPreferenceWithDefault
  • UsersManager.getAllUsersPreferences
  • VisitsSummary.getColumns

If any of those methods shouldn't be moved, let me know.

Otherwise won't do that in one big PR, but in one PR per plugin or so. That should make it easier and faster to review.

@sgiehl sgiehl self-assigned this Apr 21, 2020
This was referenced Apr 21, 2020
Merged
Merged
Merged
@tsteur
Copy link
Member Author

tsteur commented Apr 21, 2020 •
edited

I'm now wondering how important it actually is. These methods can't be called through the API right? It doesn't seem all that important and I reckon we could close the issue if they can't be called directly through the API.

@sgiehl
Copy link
Member

sgiehl commented Apr 22, 2020

They can't be called directly. Nevertheless it might not make any sense to have public methods in an API class that should not be callable. But if it's not important right now, I'll stop working on it for now.
Feel free to close the issue or move it to a later milestone...

@sgiehl sgiehl modified the milestones: 4.0.0, 4.0.0 RC Apr 22, 2020
@sgiehl sgiehl removed their assignment Apr 22, 2020
@sgiehl sgiehl mentioned this issue Apr 22, 2020
Merged
@tsteur
Copy link
Member Author

tsteur commented Apr 22, 2020

Agreed it doesn't make sense but it's also not any important. It really doesn't make any difference at the end of the day. Will close it.

@tsteur tsteur closed this as completed Apr 22, 2020
@tsteur tsteur added wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it. and removed Task Indicates an issue is neither a feature nor a bug and it's purely a "technical" change. labels Apr 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
wontfix If you can reproduce this issue, please reopen the issue or create a new one describing it.
Projects
None yet
Milestone
4.0.0-RC
Development

No branches or pull requests
3 participants
@tsteur @mattab @sgiehl