-
-
Notifications
You must be signed in to change notification settings - Fork 389
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
Create API for map pins search #3955
Create API for map pins search #3955
Conversation
onearmy-community-platform Run #6479
Run Properties:
|
Project |
onearmy-community-platform
|
Branch Review |
pull/3955
|
Run status |
Passed #6479
|
Run duration | 05m 24s |
Commit |
a6768f09e1: fix: fixed lint issues
|
Committer | unknown |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
1
|
Pending |
0
|
Skipped |
0
|
Passing |
75
|
View all changes introduced in this branch ↗︎ |
src/routes/api.mappins.$id.ts
Outdated
// taken from platform-api | ||
async function getUserProfile(userId) { | ||
const collectionRef1 = collection(firestore, DB_ENDPOINTS.users) | ||
const usersQuery1 = query(collectionRef1, where('_authId', '==', userId)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benfurber do you know why we need to query for _authId?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe _authId relates to the auth stuff which is linked to a user recond on sign-up. Not sure why it's needed here...
src/routes/api.mappins.ts
Outdated
// check if cached map pins are availbe, if not - load from db and cache them | ||
if (cachedMappins) { | ||
return json({ mapPins: cachedMappins }) | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need the else since the if returns
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed :)
src/routes/api.mappins.$id.ts
Outdated
return queryResults1.docs[0].data() | ||
} | ||
|
||
const collectionRef2 = collection(firestore, DB_ENDPOINTS.users) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if querying for _authId is really needed, the collection is the same, so we don't need a "collectionRef2"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed :)
src/routes/api.mappins.$id.ts
Outdated
const usersQuery2 = query(collectionRef2, where('_id', '==', userId)) | ||
const queryResults2 = await getDocs(usersQuery2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if querying for _authId is really needed, can still make the code more readable:
const usersCollection = collection(firestore, DB_ENDPOINTS.users);
const usersByAuthId = await getDocs(query(usersCollection, where('_authId', '==', userId)));
const usersById = await getDocs(query(usersCollection, where('_id', '==', userId)));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed :)
🎉 This PR is included in version 2.10.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
PR Checklist
PR Type
What kind of change does this PR introduce?
What is the current behavior?
map pins fetched from the old API (platfrom-api)
What is the new behavior?
map pins is fetching using Remix backend API
Does this PR introduce a breaking change?
Git Issues
Closes #3916
What happens next?
Thanks for the contribution! We try to make sure all PRs are reviewed ahead of our monthly maintainers call (first Monday of the month)
If the PR is working as intended it'll be merged and included in the next platform release, if not changes will be requested and re-reviewed once updated.
If you need more immediate feedback you can try reaching out on Discord in the Community Platform
development
channel.