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

List stores by alphabetical order if location is not given in the request #117

Merged
merged 3 commits into from
Jun 26, 2020

Conversation

redhoteggplant
Copy link
Collaborator

@redhoteggplant redhoteggplant commented Jun 24, 2020

Implemented a query limit for performance, but maybe this should be removed?
Other options for fallbacks if user doesn't share their location:

  • Require user to manually input a location to see any results (e.g. foodpanda)
  • Use a default location (e.g. GrabFood)
  • Show all or a limited number (current implementation) of stores, ordered by alphabetical order

Unresolved: jest continues to detect open handles, as described in greater detail in #100

@devYaoYH
Copy link
Collaborator

Did you pull #104 which temporarily fixed the server test ordering issue?

@redhoteggplant redhoteggplant marked this pull request as draft June 24, 2020 08:38
Copy link
Collaborator

@cszqwe cszqwe left a comment

Choose a reason for hiding this comment

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

Fix the failed test cases.

server/controllers/stores-controller.js Outdated Show resolved Hide resolved
server/tests/routes.test.js Outdated Show resolved Hide resolved
@redhoteggplant redhoteggplant marked this pull request as ready for review June 24, 2020 12:54
@@ -38,11 +38,6 @@ app.use((err, req, res, next) => {
app.use(compression());

const server = app.listen(PORT, function () {
console.log(config.ENV);
if (config.ENV === config.TEST) {
Copy link
Collaborator

@devYaoYH devYaoYH Jun 24, 2020

Choose a reason for hiding this comment

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

To clarify: I intended this to be here for testing with yarn start_test which is set up to use the emulated firestore DB rather than our live DB. But maybe there is a better place to put this initialization code on server startup.

@redhoteggplant redhoteggplant merged commit 8f3a449 into googleinterns:master Jun 26, 2020
redhoteggplant added a commit to redhoteggplant/scan-and-go that referenced this pull request Jun 30, 2020
List stores by alphabetical order if location is not given in the request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants