Rethink parsing and generation of urls #274

Open
opened 2019-11-19 10:42:39 +00:00 by matthijskooijman · 1 comment
matthijskooijman commented 2019-11-19 10:42:39 +00:00 (Migrated from github.com)

URL parsing currently happens in HyphaRequest and has already been improved a lot. However, there might still be things to improve here (e.g. for removing duplicate code in dewikifying in #147 or making things a bit more readable perhaps).

On the other hand, url generation is quite ad-hoc at the moment. With the ongoing datatype refactor, part of this generation is handled in constructFullPath methods in the datatypes, but that should probably be handled in a more central place (and the method should get a better name, too).

Some considerations:

  • It would be better to handle urls as arrays of urls components and do the join-by-/ in a central place, rather than doing string concatenation everywhere.
  • It should be easier to generate urls within the current page (e.g. to pass in ['edit'] and get /<current_language>/<current_page>/edit), generate an url to another page without knowing how the url scheme works exactly.
  • It might be good to have a tighter coupling between url parsing and generation, similar to what Django does (which has url patterns that are used for mapping urls to views, and can be reversed to generate an url to a view). This does not really fit our current url parsing model, but might be interesting to keep in mind. The url generation part of this is a bit what we're doing in article right now:
    PlanBCode/hypha@be51f688ed/system/datatypes/peer_reviewed_article.php (L76)
    PlanBCode/hypha@be51f688ed/system/datatypes/peer_reviewed_article.php (L710)
  • It should be possible to parse an url from a string, rather than the server variables (maybe without creating a full HyphaRequest object), e.g. for #147.
  • Special characters in url components (e.g. spaces in tags) should be handled better by consistent rawurlencode / rawurldecode.
URL parsing currently happens in `HyphaRequest` and has already been improved a lot. However, there might still be things to improve here (e.g. for removing duplicate code in dewikifying in #147 or making things a bit more readable perhaps). On the other hand, url generation is quite ad-hoc at the moment. With the ongoing datatype refactor, part of this generation is handled in `constructFullPath` methods in the datatypes, but that should probably be handled in a more central place (and the method should get a better name, too). Some considerations: - It would be better to handle urls as arrays of urls components and do the join-by-/ in a central place, rather than doing string concatenation everywhere. - It should be easier to generate urls within the current page (e.g. to pass in `['edit']` and get `/<current_language>/<current_page>/edit`), generate an url to another page without knowing how the url scheme works exactly. - It might be good to have a tighter coupling between url parsing and generation, similar to what Django does (which has url patterns that are used for mapping urls to views, and can be reversed to generate an url to a view). This does not really fit our current url parsing model, but might be interesting to keep in mind. The url generation part of this is a bit what we're doing in article right now: https://github.com/PlanBCode/hypha/blob/be51f688ed569a8276050a146b410a23e618200b/system/datatypes/peer_reviewed_article.php#L76 https://github.com/PlanBCode/hypha/blob/be51f688ed569a8276050a146b410a23e618200b/system/datatypes/peer_reviewed_article.php#L710 - It should be possible to parse an url from a string, rather than the server variables (maybe without creating a full HyphaRequest object), e.g. for #147. - Special characters in url components (e.g. spaces in tags) should be handled better by consistent `rawurlencode` / `rawurldecode`.
matthijskooijman commented 2020-04-23 12:33:22 +00:00 (Migrated from github.com)

I added a point about urlencoding/decoding in the first post. This came up with the tagindex, which now manually urldecodes, but this should really be handled by HyphaRequest.

I added a point about urlencoding/decoding in the first post. This came up with the tagindex, which now manually urldecodes, but this should really be handled by HyphaRequest.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
harmen/hypha#274
No description provided.