proposal: change to list_detours and other detour_admin endpoint to request fields explicitly
#2948
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ns.Db.Detour.Queries` into `Skate.Detours.Db.Detour.Queries`
wip! working, pr ready wip! add virtual fields to struct wip! add route id
firestack
commented
Jan 29, 2025
lib/skate/detours/db/detour.ex
Outdated
Comment on lines
120
to
140
| |> preload([ | ||
| :author | ||
| ]) |
There was a problem hiding this comment.
note: I feel like there should be a better way to tell if we need to preload the author, (and preload using a join instead) but I wasn't able to figure it out in time.
There was a problem hiding this comment.
assuming you mean you want to have the preload([:author]) statement present if a condition is met, you can do something like.....
defp preload_author(query, thing_to_check) do
if thing_to_check == true do
preload(query, [:author])
else
query
end
endwip! make `with_fields` handle virtual fields wip! move split_fields into own function review! wip! replace `with_author`
firestack
force-pushed
the
kf/wip/proposed-query-changes
branch
4 times, most recently
from
373a6b8 to
2edd643
Compare
firestack
changed the title
list_detours and other Read API endpointslist_detours and other detour_admin endpoint to request fields explicitly
firestack
force-pushed
the
kf/wip/proposed-query-changes
branch
from
2edd643 to
8eefc7a
Compare
firestack
force-pushed
the
kf/wip/proposed-query-changes
branch
from
8eefc7a to
0b8995d
Compare
bfauble
reviewed
Jan 31, 2025
There was a problem hiding this comment.
more of a general question for discussion, why do we have Detailed, ActivatedDetourDetails, and WithState structs all for Detour?
especially in the case of ActivatedDetourDetails it seems excessive to add 2 fields and add an additional complexity of nesting the actual Detour info within :details
bfauble
approved these changes
Feb 4, 2025
firestack
force-pushed
the
kf/asn/json-query
branch
from
d0eb72d to
45462f9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some ideas on how we should structure the "Read" subset of CRUD API's in Skate, but specifically for Detours in this PR.
I think this could be merged into #2946 if people like it.