@diosmosis opened this Pull Request on September 25th 2018 Member

Changes:

I am not sure if this fix will have repercussions so I haven't modified the tests. Will do so after someone else gives their opinion.

Fixes #13460

@tsteur commented on September 25th 2018 Member

If it fixes your issue and tests pass probably all good to go. It's almost impossible to review it since the segments are just a mess and we never know where they are urlencoded or urldecoded etc.

@diosmosis commented on September 25th 2018 Member

It will change a lot of the test output I think (another encoding in the segment property in API output). Think this won't be a BC break? at least not a potentially huge one.

@tsteur commented on September 25th 2018 Member

Think this won't be a BC break?

I really don't know. As long as we can still handle the segments urlencoded and urldecoded it should be fine.

@fdellwing commented on September 26th 2018 Contributor

Applied the patch locally and it resolves the issue for me.

@tsteur commented on December 6th 2018 Member

https://github.com/matomo-org/matomo/issues/13460#issuecomment-423856472

Don't have a fix yet, but found the cause of the error. Actions are stored urlencoded in the log_action table, but the value in the segment when opening the segmented visitor log/transitions modal/etc. is not.

I'm not sure this is the case? That might have been the case quite a while ago but shouldn't be the case anymore? at least for me it was stored normally
image

or you mean the URL is encoded after tracking?

@tsteur commented on December 6th 2018 Member

I've looked at the PR for quite a while but the PR is quite complex and not trivial to understand especially re side effects etc. Left some comments though. Do you think there would be otherwise a solution where we don't need to change anything except for maybe the segment expression?

@diosmosis commented on December 13th 2018 Member

I'm not sure this is the case? That might have been the case quite a while ago but shouldn't be the case anymore?

It's not the case, it looked like it was, but I think I tracked something wrong.

@mattab commented on May 28th 2019 Member

@diosmosis Feedback:

  • Would it be possible to create a failing UI tests in 3.x-dev branch (in a separate PR) which shows the bug(s) currently in Matomo
  • Then could you update this PR and solve conflicts and show that the bug(s) are now fixed.

Looking forward to merging this :+1:

@diosmosis commented on May 28th 2019 Member

This change turned out to be rather complex, I doubt I'll be able to finish it soon, and it's not as high value as other tickets. Would vote to put it in the priority backlog.

@mattab commented on June 10th 2019 Member

@diosmosis :ok_hand: But be good to finish this later (before Matomo 4 ideally...). Moving it to 3.12.0 milestone for now.

Powered by GitHub Issue Mirror