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

Aaf 110 donors filter #126

Merged
merged 11 commits into from
Jul 10, 2018
Merged

Aaf 110 donors filter #126

merged 11 commits into from
Jul 10, 2018

Conversation

phuchle
Copy link
Contributor

@phuchle phuchle commented Jul 8, 2018

This PR adds the routes api/donors and /api/donors/:filter.

To show all donors, make a GET request to api/donors with no query fields.
To filter by one field:
GET: /donors/_id?value=123 // Returns array of donors matching or empty array if no matches

@phuchle phuchle changed the base branch from development to AAF-63-models July 8, 2018 01:11
@jwu910 jwu910 requested review from psyduc, rainrivas and jwu910 July 8, 2018 01:59
Copy link
Contributor

@jwu910 jwu910 left a comment

Choose a reason for hiding this comment

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

Theres a merge conflict that should be resolved. Probably rebase on to AAF-63-models first. Or just fix conflict in GUI .

I'm good with all this. I think I'm gonna need some time to really crawl through everything we've done again.

Copy link
Contributor

@rainrivas rainrivas left a comment

Choose a reason for hiding this comment

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

Travis looks to be failing due to some issue downloading it's own dependencies so I don't see how it would be a problem for the test executions, however, the conflict needs to be resolved first. Aside from that warning, everything does look good to me.

Merge branch 'AAF-63-models' of github.com:5-gwoap/adopt-a-family into AAF-110-donors-filter
@jwu910
Copy link
Contributor

jwu910 commented Jul 9, 2018

Just started reviewing :)

:octocat: Sent from GH.

@jwu910
Copy link
Contributor

jwu910 commented Jul 9, 2018

Latest build is still failing due to i think a bcrypt min version requirement for latest node.

https://github.com/kelektiv/node.bcrypt.js#version-compatibility

Should we upgrade to >=3.0?

@rainrivas
Copy link
Contributor

rainrivas commented Jul 9, 2018

@jwu910 I am not so sure that is the fix, but I'm down to try. bcrypt page on npm says a few different things...


Node Version Bcrypt Version
0.4.x <= 0.4.x
0.6.x >= 0.5.x
0.8.x >= 0.5.x
0.10.x >= 0.5.x
0.11.x >= 0.8.x
4.x.x <= 2.0.x
8.x.x >= 1.0.3
10.x.x >= 3.0.0

node-gyp only works with stable/released versions of node. Since the bcrypt module uses node-gyp to build and install you'll need a stable version of node to use bcrypt. If you do not you'll likely see an error that starts with:

gyp ERR! stack Error: "pre" versions of node cannot be installed, use the --nodedir flag instead

EDIT:: We could create a TODO to update bcrypt to v3.0 anyway since it says that would support everything. Currently we are at ^1.x.y so we would never make it to even 2.0; we need to make sure our code is compatible with 3.x

@phuchle
Copy link
Contributor Author

phuchle commented Jul 9, 2018

@rainrivas We probably need a PR that updates a lot of the packages, since GH is warning about vulnerabilities .

@jwu910
Copy link
Contributor

jwu910 commented Jul 9, 2018

@phuchle Yeah, figured we could do that, I commented on the dependency for this PR just cause Travis was erring one of its builds. Not sure what the best course of action was

Copy link
Contributor

@jwu910 jwu910 left a comment

Choose a reason for hiding this comment

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

Gucci. Lgtm

@phuchle phuchle merged commit d5ccd14 into AAF-63-models Jul 10, 2018
@phuchle phuchle deleted the AAF-110-donors-filter branch July 10, 2018 03:22
rainrivas pushed a commit that referenced this pull request Jul 10, 2018
* Closing #30 by connecting to Heroku (#53)

* Uncommenting logic to use build folder. Also using double quotes to check travis behavior

* Adding `build` to source control

* Adding Procfile to have heroku run a specific server command

* Mofidying the path for express public folder

* Modyfing homepage field on package.json and pushing up new build that use the homepage url

* Adding build folder to eslint ignore file so we do not fail CI. Also removing our build folder from .gitignore so we can easily push up the build.

* Install mongoose and bcrypt for writing models.

* Add base user model and placeholder files for donor, family, and organizer models.

* Add more fields and refine methods on userSchema.

* Run user.js through prettier.

* Add wishlist schema.

* Refine existing models for consistency and clarity. Add initial version of other base models.

* Add a few more methods before realizing I have no idea what I'm doing.

* Update models.  Add donor and family controllers. Add body-parser package for dealing with form data.

* Setting up a very basic set of api routes

* Adding basic logic for /pairing route. Modifying Donor and Family schema
to make references to each otehr for matching and adding default values
to it. /pairing should find all donors and filter them based on budget then sort budgets in descending order.

* Adding organizer to the donor and family models

* Adding pseudo logic for /paring/balance route

* Adding notes to /pairing/balance to remind us to use the readWishlist() method

* Correcting issues with api.js

Switching the min/max budgets in the LTE/GTE query and removing the p from "res"

* SeedDB script is working now (#93)

* Fix capitalization error in server/models.

* Update ecmaVersion to latest in order to use async/await.

* Implement working seedDB script by making liberal use of async/await.

* Add consistent error/sucess output on saving each model.

* Aaf 97 budget pairing (#101)

* Fixing #100 - Removing extra key, field, from Donor.sort

* Pulling the data from the request correctly so we can find budgets accordingly

* Fixing #97. The max budget cap is not implemented so we sort at a +/- 10% in ascending order for now. Also did some work for setting the database route to 'aaf' in our api file. Also setting matchedFamily to null in our models and correcting the name mapping in the db seed file.

* Correcting the sort order to be as #97 is asking.

* Add wishlist seeding to seedDB script (#102)

Add wishlist seeding to seed DB and refine error handling for wishlists and families.

* AAF-104 Change body parser declaration from app to routes

* Api test setup (#106)

* WIP :: Setting up API tests with base skeleton

* Modifying app.js to export the app.listen so that the tests can run correctly. Setting up a single happy path test for /api

* Enhancing api tests to make proper assertions.

* Adding logic to /api/pairing to handle missing budget params. Making tests more relevant to pairing route.

* Adding tests for making sure messages are only should when the appropriate number of donors are returned

* Removing duplicated bodyparser line in api.js

* Cleaning up the Pairing test to split the tests into sub-sets. Also adding some basic wishlist test templates for the wishlist routes.

* Removing the babel dependencies that were added since they aren't actually used in testing.

* Fixing the issue where only one test would run. Also, since multiple tests can no run you can watch the api tests using $(npm bin)/jest api --watch
This assumes you're naming api tests as <ROUTE>Api.test.js

* Removing useless comment on app.js

* Fixing the linter issue for wishlist route so npm test will execute

* Fixing the unused imports issue

* AAF-111 Add usePushEach as deprecation workaround for mongo that affects array push methods (#112)

* AAF-111 Add usePushEach as deprecation workaround for mongo that affects array push methods

* AAF-111 Npm update and package lock

* AAF-111 Add jest-cli as dev dependency

* AAF-94 wishlist api read (#108)

* add first route for families to read their wishlist

* Add more error handling.

Add a family with an empty wishlist.

* AAF94 Add a description for the regex
Fix the logic for the if statement
Change the response status to 400 and made the output messages more discriptive

* AAF 94 Change the HTTP code for no family found to 404.

* AAF-94 Fix uppercase for const variable

* AAF-94 Revert the change to the familyId variable and fix the route

* AAF-94 Remove the delete route

* AAF-113 npm test api -- Fix #113 (#114)

*  Trying to fix #113 by using concurrently to run all the tests but I doubt that this will work correctly. I might have to make the changes at the travis.yml file

* (#113)Telling travis to cache node_modules so that it doesn't have to constantly install if things don't change

* 113 - Skipping the wishlist tests since they aren't actually written. However, when travis runs concurrently, it might be failing to make a second connection to mongo.. will need to test this

* AAF#113 - Adding mongodb service and seed script to travis.yml to solve the issue with scripts not running.

* AAF-#113 - Skipping the POST routes tests in the wishlist API

* Removing node 4 and 6 from Travis since we won't be supporting that in prod. Also modifying our README to show system requirements that are recommending a minium node version of 7 and a mongodb version of 3.6 (#113)

* Modifying the test:api command to now run on a node nevironment since we're testing API (node services) and not some dom elements. Also setting API test to trigger OS Notification and to force quit once the execution is done ( no watch mode ) (#113)

* moved the totalListCost outside of the list array (#117)

* Aaf 109 families filter (#122)

AAF-109 Add families filter route.

This route is expected to be used like so:

- GET /api/families with no queries returns list of all families

- GET /api/families?filter=_id&value=123
Passing filter and value will look up families with the provided filter and value and return an object with a property families, an array of the families.

If you pass less than 2 or more than 2 query fields, the route throws 400.

If you pass exactly 2 query fields but they are not filter or value, it will throw 400.

eg: filter: foo and valu: bar or foo: bar and bar: foo

* Aaf 110 donors filter (#126)

* AAF-109 Write show and filter route for donor.  Experimenting with controller abstraction. 

* AAF-110 Rebuild bcrypt and node-sass due to errors that caused npm start to break.

* Removing bcrypt from our package.json since we do not need this package... i dont think

* readding bcrypt as version 2.0.1 since we are using it in seed file.

* Setting up Bcrypt v3.0

* AAF-63 Update node-sass-chokidar. 1.3.0 released. node-sass dependencies should build with node-gyp

* AAF-63 Autogenerate packagelock
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.

4 participants