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

Fix for unhandled datatable maps in custom dimensions #20044

Merged
merged 3 commits into from Nov 28, 2022

Conversation

bx80
Copy link
Contributor

@bx80 bx80 commented Nov 23, 2022

Description:

If the Archive::createDataTableFromArchive() method returns a DataTable\Map instead of a DataTable object then getCustomDimension() tries to use it as a single DataTable object resulting in errors. This fix checks if the returned object is a DataTable\Map and if so uses a different method to retrieve the required data.

Fixes #20043

Review

@bx80 bx80 added the Bug For errors / faults / flaws / inconsistencies etc. label Nov 23, 2022
@bx80 bx80 added this to the 5.0.0 milestone Nov 23, 2022
@bx80 bx80 self-assigned this Nov 23, 2022
@bx80 bx80 changed the base branch from 4.x-dev to 5.x-dev November 23, 2022 20:39
@justinvelluppillai
Copy link
Contributor

You can probably base this off 4.x-dev and aim for 4.13.0

@bx80 bx80 changed the base branch from 5.x-dev to 4.x-dev November 23, 2022 22:57
@bx80 bx80 modified the milestones: 5.0.0, 4.12.6 Nov 23, 2022
@bx80 bx80 added the Needs Review PRs that need a code review label Nov 24, 2022
@sgiehl
Copy link
Member

sgiehl commented Nov 24, 2022

@bx80 When changing the base branch of the PR, you need to rebase the branch on the new base branch. Merging this branch in won't remove all the additional commits from the original base. If you need help with that, let me know.

@bx80
Copy link
Contributor Author

bx80 commented Nov 25, 2022

@sgiehl Thanks, I've rebased the branch from 4.x-dev and force pushed the changes, it's still showing extra commits though. It's fairly painful to rebase 5.x-dev to 4.x-dev at the moment with the Angular removal changes. Might be cleaner if I just create a new branch / PR?

@sgiehl
Copy link
Member

sgiehl commented Nov 25, 2022

@bx80 rebasing should only be painful when you have dozens of merge conflicts. But in this case there shouldn't be any conflicts. So it should be a simple rebase on the branch, while removing all commits that don't belong to this PR...

Copy link
Member

@sgiehl sgiehl left a comment

Choose a reason for hiding this comment

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

I've rebased the branch, and removed all the commits left over from 5.x-dev.
If tests are passing, this one should be good to merge.

@sgiehl sgiehl removed the Needs Review PRs that need a code review label Nov 28, 2022
@bx80
Copy link
Contributor Author

bx80 commented Nov 28, 2022

Thanks @sgiehl! 😃 I was mistakenly attempting to merge in the other 5.x commits rather than just removing them 🙄

@bx80 bx80 merged commit 3f02519 into 4.x-dev Nov 28, 2022
@bx80 bx80 deleted the m20043-custom-dim-getrows branch November 28, 2022 21:38
@justinvelluppillai justinvelluppillai modified the milestones: 4.12.6, 4.13.0 Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fatal error: Call to undefined method Piwik\\DataTable\\Map::getRows() in CustomDimensions/API.php
3 participants