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

JIP-20 Filter posts by location #17

Merged
merged 18 commits into from Jul 16, 2021
10 changes: 7 additions & 3 deletions packages/client/src/pages/home/map.tsx
Expand Up @@ -7,7 +7,8 @@ import {
} from "@react-google-maps/api";
import { CSSProperties } from "@material-ui/styles";
import { Libraries } from "@react-google-maps/api/dist/utils/make-load-script-url";
import { Location, setLocation } from "../../redux/slices/location-slice";
import { Location } from "../../redux/slices/location-slice";
import { setLocationFilter } from "../../redux/slices/post-slice";
import REACT_APP_GOOGLE_API_KEY from "../../env";
import styled from "styled-components";
import { useAppDispatch, useAppSelector } from "../../redux/store";
Expand Down Expand Up @@ -58,15 +59,15 @@ const onAutoCompleteLoad = (autocomplete: any) => {

const Map = () => {
const dispatch = useAppDispatch();
const location = useAppSelector((state) => state.location);
const location = useAppSelector((state) => state.post.locationFilter);
const updateStore = async (
lat: number,
lon: number,
newName: string | undefined,
) => {
try {
const response = await dispatch(
setLocation({
setLocationFilter({
name: newName !== undefined ? newName : name,
lat: lat,
lon: lon,
Expand All @@ -84,9 +85,12 @@ const Map = () => {
lat: location.lat,
lng: location.lon,
});

const [name, setName] = React.useState(location.name);

if (initialLocation === undefined) {
// TODO: this block is causing getPostsByFilter to be called twice when the page is refreshed
Copy link
Owner

Choose a reason for hiding this comment

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

Should we just keep lat/lon in the store rather than both?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wanted to but I had an issue with needing the map to update the post-list when I tried keeping it in the store 🐵

// Another issue: user location is fetched here AND in post-slice
Copy link
Collaborator

@dryu99 dryu99 Jul 15, 2021

Choose a reason for hiding this comment

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

Assuming this code block only needs to run once, to address these two issues maybe we could:

  • Get rid of the location slice so the current location is only maintained in one place (i.e. in the post-slice).
  • Implement an action in the post-slice called initLocation that does sth like
navigator.geolocation.getCurrentPosition(function (position) {
  state.locationFilter = {
    lat: position.coords.latitude,
    lon: position.coords.longitude,
    name: undefined,
  };
});
  • Dispatch initLocation in a useEffect hook in HomePage or Map (to avoid redundant calls/renders)
  • Or we could just call the above code block in a useEffect hook and set it in the store (no additional action necessary)

We could also keep the location-slice and just have all location-related store logic stay in there (which implies we remove location logic from post-slice)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I did it the second way and it fixes most of the problem

Just noticed that we also called getPostsByFilter for the initialLocation, but at least the duplicate requests for the detected location are gone 🐵 🐵

navigator.geolocation.getCurrentPosition(function (position) {
initialLocation = {
lat: position.coords.latitude,
Expand Down
15 changes: 11 additions & 4 deletions packages/client/src/pages/home/post-list/post-list.tsx
@@ -1,8 +1,12 @@
import React, { useEffect } from "react";
import { connect } from "react-redux";
import styled from "styled-components";
import { useAppDispatch, useAppSelector } from "../../../redux/store";
import PostListItem from "./post-list-item";
import { getPosts, PostSortType } from "../../../redux/slices/post-slice";
import {
getPostsByFilter,
PostSortType,
} from "../../../redux/slices/post-slice";

const StyledContainer = styled.div`
display: flex;
Expand Down Expand Up @@ -30,13 +34,16 @@ const PostList: React.FC = () => {
default:
console.log("damn wtf");
}

return result;
});

const postState = useAppSelector((state) => state.post);

const location = useAppSelector((state) => state.post.locationFilter);

useEffect(() => {
dispatch(getPosts());
}, [dispatch]);
dispatch(getPostsByFilter(postState));
}, [location]);

return (
<StyledContainer>
Expand Down
48 changes: 40 additions & 8 deletions packages/client/src/redux/slices/post-slice.ts
@@ -1,16 +1,25 @@
import { createAsyncThunk, createSlice, PayloadAction } from "@reduxjs/toolkit";
import postService from "../../services/posts";
import { Location } from "./location-slice";
import { Location, initialState as initialLocation } from "./location-slice";
import { UserState } from "./user-slice";

const prefix = "post";

type Comment = {
_id: string;
date: string;
numUpVotes: number;
numDownvotes: number;
userId: string;
username: string;
};

export interface Post extends NewPost {
_id: string;
numUpvotes: number;
numDownvotes: number;
date: string;
comments: string[];
comments: Comment[];
username: string;
}

Expand All @@ -27,22 +36,28 @@ export enum PostSortType {
NEW = "new",
}

type PostState = {
export type PostState = {
posts: Post[];
sortType: PostSortType;
locationFilter: Location;
tagFilter?: string;
currentPostID?: string;
};

let locationFilter: Location = initialLocation;

navigator.geolocation.getCurrentPosition(function (position) {
locationFilter = {
lat: position.coords.latitude,
lon: position.coords.longitude,
name: undefined,
};
});

const initialState: PostState = {
posts: [],
sortType: PostSortType.POPULAR,
locationFilter: {
name: "Vancouver",
lat: 49.26,
lon: -123.22,
},
locationFilter: locationFilter,
};

export const createPost = createAsyncThunk<Post, NewPost>(
Expand All @@ -63,7 +78,18 @@ export const getPosts = createAsyncThunk<Post[]>(
async (_, { rejectWithValue }) => {
try {
const response = await postService.getAll();
return response.data;
} catch (error) {
return rejectWithValue(error);
}
},
);

export const getPostsByFilter = createAsyncThunk<Post[], PostState>(
`${prefix}/getPostsByFilter`,
async (postState: PostState, { rejectWithValue }) => {
try {
const response = await postService.getPostsByFilter(postState);
return response.data;
} catch (error) {
return rejectWithValue(error);
Expand Down Expand Up @@ -142,6 +168,12 @@ export const postSlice = createSlice({
builder.addCase(getPosts.rejected, (state, action) => {
return { ...initialState };
});
builder.addCase(getPostsByFilter.fulfilled, (state, action) => {
state.posts = action.payload;
});
builder.addCase(getPostsByFilter.rejected, (state, action) => {
return { ...initialState };
});
builder.addCase(createPost.fulfilled, (state, action) => {
state.posts.push(action.payload);
});
Expand Down
17 changes: 16 additions & 1 deletion packages/client/src/services/posts.ts
@@ -1,5 +1,5 @@
import axios from "axios";
import { NewPost, Post } from "../redux/slices/post-slice";
import { NewPost, Post, PostState } from "../redux/slices/post-slice";

const baseUrl = "/api/posts";

Expand All @@ -21,6 +21,20 @@ const getAll = async () => {
return response;
};

const getPostsByFilter = async (postState: PostState) => {
const { posts, sortType, locationFilter, tagFilter, currentPostID } =
postState;
const response = await axios.get(
`${baseUrl}?` +
`sort=${sortType}&` +
Copy link
Owner

Choose a reason for hiding this comment

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

axios does query params for you, just pass all of that inside an object with a params field. https://github.com/axios/axios#example

You don't even have to worry about url encoding each component

`lat=${locationFilter.lat}&` +
`lon=${locationFilter.lon}&` +
`tag=${tagFilter}&` +
`id=${currentPostID}`,
);
return response;
};

const getByID = async (id: string) => {
const response = await axios.get(`${baseUrl}/${id}`);
return response;
Expand All @@ -39,6 +53,7 @@ const deleteByID = async (id: string) => {
const postService = {
create,
getAll,
getPostsByFilter,
getByID,
update,
deleteByID,
Expand Down
2 changes: 1 addition & 1 deletion packages/server/setup/datagen.js
Expand Up @@ -201,7 +201,7 @@ class DataGenerator {
const rawLocations = fs.readFileSync(this.locationPath);
this.mockLocations = JSON.parse(rawLocations).map((location) => ({
type: "Point",
coordinates: [location.lat, location.lng],
coordinates: [location.lng, location.lat],
Copy link
Owner

Choose a reason for hiding this comment

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

Wack literally everywhere else its lat first.

}));
const rawComments = fs.readFileSync(this.commentsPath);
this.mockComments = JSON.parse(rawComments);
Expand Down