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

Use https for urls in visitor details if host is defined with https in site #17151

Merged
merged 7 commits into from Mar 11, 2021

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Jan 27, 2021

Description:

refs #5312, #12586

Review

  • Functional review done
  • 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

@sgiehl sgiehl added the Needs Review PRs that need a code review label Jan 27, 2021
@sgiehl sgiehl added this to the 4.2.0 milestone Jan 27, 2021
tsteur
tsteur previously requested changes Jan 28, 2021
Copy link
Member

@tsteur tsteur left a comment

Choose a reason for hiding this comment

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

Worked. Left 2 comments. Can you also already prepare an FAQ for this? Was wondering if we need to do the same for Actions.getPageUrls report for example maybe?

plugins/Actions/VisitorDetails.php Outdated Show resolved Hide resolved
plugins/Actions/VisitorDetails.php Outdated Show resolved Hide resolved
@sgiehl
Copy link
Member Author

sgiehl commented Jan 29, 2021

Afaik Actions.getPageUrls and reports like that are generating the urls while archiving. We could change that there or manipulate the datatable before it is returned from API.

Regarding FAQ: Not sure which question it actually should answer.

@tsteur
Copy link
Member

tsteur commented Jan 31, 2021

Regarding the FAQ: I meant something like: How do I fix links in visitor log going to HTTP instead of HTTPS? or something like this. Or "How do I change the links to my pages to use HTTPS in Matomo?"

We could change that there or manipulate the datatable before it is returned from API.

Sounds good. I think it be good to have consistent behaviour there if possible.

Copy link
Member

@diosmosis diosmosis left a comment

Choose a reason for hiding this comment

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

Left a couple comments. Tested locally and it works 👍

core/Tracker/PageUrl.php Outdated Show resolved Hide resolved
plugins/Actions/DataTable/Filter/Actions.php Show resolved Hide resolved
plugins/Actions/VisitorDetails.php Show resolved Hide resolved
@sgiehl sgiehl force-pushed the vlogprotocol branch 4 times, most recently from 1c643fe to 6ceadd6 Compare March 11, 2021 14:17
@sgiehl
Copy link
Member Author

sgiehl commented Mar 11, 2021

@diosmosis I've applied some changes. Let me know if it's good to merge now.

@sgiehl sgiehl requested a review from diosmosis March 11, 2021 16:09
@diosmosis diosmosis merged commit 96fcda9 into 4.x-dev Mar 11, 2021
@diosmosis diosmosis deleted the vlogprotocol branch March 11, 2021 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants