DEV: Add X-Discourse-BrowserPageView-URL response header
#36647
Conversation
99d5da8 to
e906210
Compare
e906210 to
039fee0
Compare
7ef2e76 to
211dae3
Compare
Reviewer notesI tested locally in development with Full page load (tracking via MessageBus)
onPageChange (tracking via first ajax request)
|
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.
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.
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.