https://github.com/matomo-org/matomo/pull/17461 - Mouve

Site d'origine

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

Add get-segment-sql development command for debugging #17461

Merged
merged 2 commits into from Apr 16, 2021

Conversation

@diosmosis
Copy link
Member

@diosmosis diosmosis commented Apr 15, 2021

Description:

Prints out the query of a segment, can be used if trying to determine if a segment is query is being built correctly or not.

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
Copy link
Contributor

@flamisz flamisz left a comment

I'm not 100% sure what is this for, but tested and got results that look ok to me 👍

@@ -240,7 +240,7 @@ public function getSiteFromId($idSite)
* Returns the list of all the website IDs registered.
* Caller must check access.
*
* @return array The list of website IDs
* @return int[] The list of website IDs

This comment has been minimized.

@sgiehl

sgiehl Apr 15, 2021
Member

That actually might not be correct. Depending on the mysql configuration it might be an array of strings. But guess we could add a cast to int in the foreach to ensure only ints are returned

This comment has been minimized.

@diosmosis

diosmosis Apr 15, 2021
Author Member

I could do int[]|string[]. As long as it's not array, it'll be more useful.

This comment has been minimized.

@sgiehl

sgiehl Apr 15, 2021
Member

Sure. no preference. Just wanted to mention that the return type can vary. Maybe casting to int would be even better, so we can rely on the return type?

This comment has been minimized.

@diosmosis

diosmosis Apr 15, 2021
Author Member

Might be more useful, but given we don't do that for any other query in Matomo (that I know of), I'm not sure it's really needed (and might make things more inconsistent).

This comment has been minimized.

@sgiehl

sgiehl Apr 15, 2021
Member

Feel free to change the doc block only then. Having more strict return types is a more global thing for sure 🙈

@diosmosis
Copy link
Member Author

@diosmosis diosmosis commented Apr 15, 2021

@flamisz segments are converted into SQL to select visits in Matomo, but for many segments the SQL is complicated and hard to imagine in your head (or mine at least), so the command prints it out so you can see how it's converted from a segment string (ie, pageUrl==whatever;browserCode=ff) to SQL for a specific table. You can also put a print statement or log in Matomo, which is what I used to do, but I found it annoying to do that.

@diosmosis diosmosis merged commit a5fd50b into 4.x-dev Apr 16, 2021
1 of 3 checks passed
1 of 3 checks passed
@github-actions
PHPCS
Details
@travis-ci
Travis CI - Branch Build Failed
Details
@travis-ci
Travis CI - Pull Request Build Failed
Details
@diosmosis diosmosis deleted the get-segment-query-command branch Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants

level de log mis à 1

Récupération du document https://github.com/matomo-org/matomo/pull/17461.

Récupération de l'URL https://github.com/matomo-org/matomo/pull/17461...

Raccourcis

Commandes

Fermer