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

Remove legacy api #129

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft

Remove legacy api #129

wants to merge 10 commits into from

Conversation

t-my
Copy link
Collaborator

@t-my t-my commented Oct 21, 2024

No description provided.

:body (core/save-ptv-integration-definitions db search identity body-params)})}}]]

;; legacy routes
["/v1/api"
Copy link
Contributor

@vharmain vharmain Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's currently a top-level endpoint /rest that nginx captures and proxies to the legacy-api. I think we could just swap in this thing with the path /rest/api once it's ready and remove the nginx rule. https://github.com/lipas-liikuntapaikat/lipas/blob/master/nginx/proxy.conf#L141

Comment on lines +9 to +12
(defn fill-properties [m]
(reduce (fn [acc k] (assoc-in acc [:props k] (prop-types/all k)))
m
(-> m :props keys)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could just (assoc m :props (select-keys prop-types/all (keys (:props m))))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.. or (update m :props #(select-keys prop-types/all (keys %)))

(-> m
(select-keys keys-vec)
(localize lang)
utils/->camel-case-keywords))
Copy link
Contributor

@vharmain vharmain Nov 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if it was this simple. ;) However, the differences between lipas and legacy-api are not 1:1 kebab-case -> camelCase... there are a few edge-cases and exceptions that are covered in this mapping: https://github.com/lipas-liikuntapaikat/lipas/blob/master/webapp/src/clj/lipas/integration/old_lipas/sports_site.clj#L13

...and it's inverted version https://github.com/lipas-liikuntapaikat/lipas/blob/master/webapp/src/clj/lipas/integration/old_lipas/sports_site.clj#L203

You can use rename-keys to change "new keys" to "old keys" with the hardcoded map.


(defn collect-sport-place-types [sub-category-type-code]
(->> (vals types/all)
(filter #(= (% :sub-category) sub-category-type-code))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor thing, but you can change filter to just a map lookup. We already have types indexed by sub-category (and also by main-category) here https://github.com/lipas-liikuntapaikat/lipas/blob/remove-legacy-api-v2/webapp/src/cljc/lipas/data/types.cljc#L23-L24

(defn- collect-subcategories [type-code locale]
(->>
(vals types/sub-categories)
(filter #(= (% :main-category) (str type-code)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above :)

@@ -791,7 +791,7 @@
(let [locale (or (-> req :parameters :query :lang keyword) :en)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Legacy-api defaults to :fi, let's not change that. :)

http://lipas.cc.jyu.fi/api/categories

@vharmain vharmain force-pushed the master branch 3 times, most recently from 5177de1 to a68127e Compare December 27, 2024 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants