[openstreetmap/openstreetmap-website] Support versions in elements multi fetch (PR #3715)

[Tom Hughes]

@tomhughes commented on this pull request.

I was wondering how on earth cgimap wound up with an API extension relative to rails and it it turns out the history is far older and more baroque than I was expecting.

The cgimap version came in with https://github.com/zerebubuth/openstreetmap-cgimap/pull/129 which also added the single object versions calls which rails does have - that claims to be implementing #1189 for cgimap but actually is using a subtly different syntax.

That is probably a rather unfortunate syntax in fact because unlike #1189 it doesn't maintain the separation between main controller methods and history controller methods which then leads to the code here having to a complicated dance with the main controller accessing history objects and also to the non-deterministic uniqification where if you ask for the current version of an object in both versioned and unversioned form you don't know which one will be returned.

Now of course they should always be the same but it's an interesting wart.

I don't see any obvious problem with the code other than the rather complex nature of the abstract parent as @mmd-osm  noted which is not ideal but then equally neither is duplicating everything.

Possibly it would better to add new tests rather than add lots of cases to the existing ones. I suspect @gravitystorm will think so though he'd probably also say we should split the existing one.



-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/3715#pullrequestreview-1477767622
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/3715/review/1477767622 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20230613/b6da1c61/attachment.htm>