Improvements to search engine and social network detection by sgiehl · Pull Request #9174 · matomo-org/matomo · GitHub
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

Improvements to search engine and social network detection #9174

Merged
merged 7 commits into from
Nov 18, 2015
Merged

Improvements to search engine and social network detection #9174

merged 7 commits into from
Nov 18, 2015

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Nov 5, 2015

Currently the detection definitions for search engines and social networks are stored in two "data files" containing big php arrays.

In order to make that more readable and easier to extend I've converted it to YAML files.

To make it possible to provide an automatic update for those definitions I've moved them to a separate repository (currently located in my account sgiehl/searchengine-and-social-definitions) and added a handling similar to the referrer-spam list.

I've also moved most of the code handling the search engine and social network detection to the Referrer plugin, as it is only used there.

NOTE: This changes will break compatibility of plugins using the Referrer.addSearchEngineUrls event to define own search engines. (e.g. sgiehl/piwik-plugin-ReferrersManager)

Any thoughts / comments?

Todo's (if we want to merge)

  • move/clone sgiehl/searchengine-and-social-definitions into piwik space
  • add composer file to new repo to make it usable with composer
  • update composer json of piwik to use new repo
  • Maybe write a small method to handle backward compatibility for Referrer.addSearchEngineUrls event (if merged before 3.0)

refs #8980

@sgiehl sgiehl added Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. c: Usability For issues that let users achieve a defined goal more effectively or efficiently. RFC Indicates the issue is a request for comments where the author is looking for feedback. labels Nov 5, 2015
mattab
mattab reviewed Nov 6, 2015
View reviewed changes
plugins/Referrers/Tasks.php
{
$url = 'https://raw.githubusercontent.com/piwik/searchengine-and-social-definitions/master/SearchEngines.yml';
$list = Http::sendHttpRequest($url, 30);
$searchEngines = SearchEngine::getInstance()->loadYmlData($list);
Copy link
Member

Choose a reason for hiding this comment

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

we should check that the downloaded files are valid, eg. array has more than 200 elements, before overwriting the list

@mattab
Copy link
Member

mattab commented Nov 6, 2015

Nice idea, looks good to me!

Maybe this could help other people contribute to the list of Search engines and social websites.

For repository name maybe we could use piwik/searchengine-and-social-list ?

Adding to 2.15.1 as it looks safe to push in this release.

@mattab mattab added this to the 2.15.1 milestone Nov 6, 2015
@sgiehl
Copy link
Member Author

sgiehl commented Nov 6, 2015

The mentioned improvements are done, I've create piwik/searchengine-and-social-list and added it on packagist.com.

I'll check if I can manage to handle backward compatibility for the mentioned event.

@sgiehl sgiehl force-pushed the searchengines_and_social_improvements branch from 62368f8 to 641d836 Compare November 7, 2015 21:45
@sgiehl sgiehl added Needs Review PRs that need a code review and removed Pull Request WIP Indicates the current pull request is still work in progress and not ready yet for a review. RFC Indicates the issue is a request for comments where the author is looking for feedback. labels Nov 7, 2015
@sgiehl
Copy link
Member Author

sgiehl commented Nov 7, 2015

Should be fine now.
Btw. I'll release a new version of sgiehl/piwik-plugin-ReferrersManager in the next days, that will work with the current and the new behaviour

@sgiehl sgiehl force-pushed the searchengines_and_social_improvements branch from 5902236 to b402f19 Compare November 18, 2015 22:43
@sgiehl
Copy link
Member Author

sgiehl commented Nov 18, 2015

just rebased on master. So if everything is fine, guess we could merge it now

mattab pushed a commit that referenced this pull request Nov 18, 2015
Merge pull request #9174 from piwik/searchengines_and_social_improvem…
1de540f 
…ents

Improvements to search engine and social network detection
@mattab mattab merged commit 1de540f into master Nov 18, 2015
@mattab mattab deleted the searchengines_and_social_improvements branch November 18, 2015 23:45
@mattab
Copy link
Member

mattab commented Nov 18, 2015

Looks good, a refactoring that improves the platform and makes code cleaner. hopefully it will increase contributions to our social & search engine lists 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
c: Usability For issues that let users achieve a defined goal more effectively or efficiently. Enhancement For new feature suggestions that enhance Matomo's capabilities or add a new report, new API etc. Needs Review PRs that need a code review
Projects
None yet
Milestone
2.16.0
Development

Successfully merging this pull request may close these issues.
2 participants
@sgiehl @mattab