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

Inconsistent trailing slashes in routes #399

Open
4 tasks
alyssadai opened this issue Jan 8, 2025 · 0 comments
Open
4 tasks

Inconsistent trailing slashes in routes #399

alyssadai opened this issue Jan 8, 2025 · 0 comments
Labels
flag:discuss Flag issue that needs to be discussed before it can be implemented. usability Issue affecting user or developer experience.

Comments

@alyssadai
Copy link
Contributor

alyssadai commented Jan 8, 2025

In the PR for:

we globally disabled redirection of trailing slashes, such that routes by default will now only work if they are accessed exactly as defined (e.g., if the /query route is defined without a trailing slash, then /query/ will not work).

As part of the issue spec, we decided that we would globally define routes without the trailing slash for convenience and URLs, e.g.:

api.neurobagel.org/query -> response
api.neurobagel.org/query/ -> no response
api.neurobagel.org/attributes/nb:Assessment -> response
api.neurobagel.org/attributes/nb:Assessment/ -> no response

However, when we then refactored the generic /attributes route into variable-specific routes in:

we accidentally reintroduced the trailing slash into the variable routers. The result is that at the moment, we are not consistent with where we expect trailing slashes:

api.neurobagel.org/query -> works
api.neurobagel.org/query/ -> doesn't work
api.neurobagel.org/assessments -> doesn't work
api.neurobagel.org/assessments/ -> works

Steps to implement

  • standardize the router definitions so that we do not expect / for the base of each route
  • test whether the root of app is reachable when path prefix has or lacks a trailing slash
  • check that custom favicon still works
  • ensure that behaviour is predictable with generic stripped path prefix NGINX config (note default merge_slashes behaviour)

NOTE: The query tool and f-API may also be affected by this change

@alyssadai alyssadai added usability Issue affecting user or developer experience. flag:schedule Flag issue that should go on the roadmap or backlog. labels Jan 8, 2025
@surchs surchs moved this to Backlog in Neurobagel Jan 9, 2025
@surchs surchs removed the flag:schedule Flag issue that should go on the roadmap or backlog. label Jan 9, 2025
@alyssadai alyssadai moved this from Backlog to Specify - Active in Neurobagel Jan 22, 2025
@alyssadai alyssadai moved this from Specify - Active to Specify - Done in Neurobagel Jan 22, 2025
@rmanaem rmanaem added the flag:discuss Flag issue that needs to be discussed before it can be implemented. label Jan 23, 2025
@surchs surchs moved this from Specify - Done to Implement - Active in Neurobagel Jan 30, 2025
@surchs surchs moved this from Implement - Active to Specify - Done in Neurobagel Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flag:discuss Flag issue that needs to be discussed before it can be implemented. usability Issue affecting user or developer experience.
Projects
Status: Specify - Done
Development

No branches or pull requests

3 participants