-
Notifications
You must be signed in to change notification settings - Fork 8.7k
DEV: Add X-Discourse-BrowserPageView-URL response header
#36647
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
base: main
Are you sure you want to change the base?
Conversation
99d5da8 to
e906210
Compare
e906210 to
039fee0
Compare
7ef2e76 to
211dae3
Compare
211dae3 to
dd89af4
Compare
| @capture = | ||
| lambda do |_status, headers, _body| | ||
| browser_page_view_response_headers << headers if headers["X-Discourse-BrowserPageView"] | ||
| end | ||
|
|
||
| ResponseCaptureMiddleware.register_response_capture(@capture) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is abit of a hack to capture response headers but I feel that this area of the code is important enough to test since accuracy is important. Therefore, I think the trade-off is worth it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree it's worth testing 💯
If you wanted to avoid the complexity of the middleware, an alternative approach could be to use Playwright's request/response interception. For example: https://github.com/discourse/discourse/blob/536acef425166a5980762148cf9b27bb30d0f3e3/spec/system/script_encoding_spec.rb#L10C2-L13C21
Since we don't need to manipulate the response here, maybe the even-simpler playwright_page.requests info will be enough: https://playwright.dev/docs/api/class-page#page-requests
dd89af4 to
929ebb6
Compare


This commit adds a
X-Discourse-BrowserPageView-URLresponse header, alongside theX-Discourse-BrowserPageViewresponse header, with the header value set to the referrer that triggered the browser page view tracking.Do note that the
HTTP_REFERERrequest does not provide us with this information due to the way we piggyback on existing ajax requests to track browser page views. The value of the request header will always be set to the document's location href.