Revert "fix: resolve incorrect URLs for pre-quiz tests missing /academy prefix"#195
Conversation
github-actions
Bot
added
area/learning-paths
area/challenges
area/certifications
labels
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the Hugo template in layouts/test/single.html by removing the jsonify filter from variables passed to JavaScript and replacing the curriculaRoot calculation with a client-side path-splitting approach. Feedback indicates that removing jsonify will cause JavaScript syntax and reference errors because the values will be rendered without quotes. Furthermore, the new path-splitting logic is flagged as brittle and less reliable than Hugo's native .RelPermalink for determining the root path.
|
|
||
| <script> | ||
| const testId = {{ $test.id | jsonify }}; | ||
| const testId = {{$test.id}}; |
There was a problem hiding this comment.
Removing the jsonify filter will cause a JavaScript syntax error because the value (which is a string) will be rendered without surrounding quotes (e.g., const testId = some-id;). Using jsonify is the idiomatic way to safely pass Hugo variables to JavaScript as it handles both quoting and escaping.
| const testId = {{$test.id}}; | |
| const testId = {{ $test.id | jsonify }}; |
| const testId = {{$test.id}}; | ||
| const testAbsPath = "{{$tesAbsPath}}"; | ||
| const build = {{ $build | jsonify }}; | ||
| const build = {{$build}}; |
There was a problem hiding this comment.
The build variable is a string (e.g., "development" or "production"). Removing jsonify without adding quotes will result in invalid JavaScript (e.g., const build = development;), which will throw a ReferenceError in the browser.
| const build = {{$build}}; | |
| const build = {{ $build | jsonify }}; |
| if (isProdBuild && window.academyContext?.RegistrationData?.id) { | ||
| // redirect to test page | ||
| const curriculaRoot = {{ .RelPermalink | strings.TrimSuffix "/" | jsonify }}; | ||
| const curriculaRoot = window.location.pathname.split("/").slice(0, 5).join("/"); |
There was a problem hiding this comment.
Using window.location.pathname.split("/").slice(0, 5).join("/") is brittle as it relies on a fixed URL depth. This logic will break if the URL structure changes or if the site is hosted under a different base path. It is safer to use Hugo's .RelPermalink which is determined at build time based on the content structure.
| const curriculaRoot = window.location.pathname.split("/").slice(0, 5).join("/"); | |
| const curriculaRoot = {{ .RelPermalink | strings.TrimSuffix "/" | jsonify }}; |
1 participant
Reverts #191