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

Migrate Routes from mongodb to PostgreSQL #61

Open
Jooms opened this issue Mar 20, 2023 · 15 comments
Open

Migrate Routes from mongodb to PostgreSQL #61

Jooms opened this issue Mar 20, 2023 · 15 comments
Assignees
Labels
backend concerns backend/API presentation

Comments

@Jooms
Copy link
Contributor

Jooms commented Mar 20, 2023

Disabled as part of #57, we need to re-enable all of the database models on the new Database technology.

@briswells briswells changed the title Migrate Models from mongodb to PostgreSQL Migrate Routes from mongodb to PostgreSQL Mar 20, 2023
@briswells
Copy link
Contributor

Disabled as part of #57, we need to re-enable all of the database models on the new Database technology.

Updated name as the models have been implemented using relational DB methodology. Routes now need to be updated to use the new tech stack. specifically Sequelize.

@briswells briswells self-assigned this Mar 20, 2023
@briswells
Copy link
Contributor

Linking issue to branch #route-conversion

@briswells briswells mentioned this issue Mar 20, 2023
1 task
@MMurtey MMurtey self-assigned this Mar 21, 2023
@hamsaraj7106 hamsaraj7106 self-assigned this Mar 21, 2023
@briswells
Copy link
Contributor

Looks like some of the routes I thought were updated are still having some issues. Here is the list of ones I know need some work but there may be more.

/home <- I commented out some code for expiration date that needs to be converted
/donor <- Throws an error.
/stock <- appears to be working but a 2nd set of eyes would be great.
/add_stock <- appears to be working but a 2nd set of eyes would be great.
/checkout <- Needs to be retrofitted.
/checkout <- Needs to be retrofitted.
/charts <- Needs to be retrofitted.

@AbhinavReddy-Dev AbhinavReddy-Dev self-assigned this Mar 21, 2023
@AbhinavReddy-Dev
Copy link
Contributor

I would like help out anyone new to backend development trying to work on the routes, please let me know.

@MMurtey
Copy link
Contributor

MMurtey commented Mar 23, 2023

I'll start with /donor.

@briswells
Copy link
Contributor

I'll start with /donor.

I'm pretty sure one of the issues with that route is "undefined" no used in postgressql. Besides that the findall should be good. idk about the rest of the code though.

@briswells
Copy link
Contributor

I was just looking at the /charts route and from what I can see all the math is handled by the front end super inefficiently. I added a comment to the React discussion thread asking if we can shift this to the backend during the rewrites, so we should probably hold off and rewriting it until we've come to a consensus.

@hamsaraj7106
Copy link
Contributor

I'll work on the commented part of the code in /home

@MMurtey
Copy link
Contributor

MMurtey commented Mar 24, 2023

/donor query is now pulling all "person"s with a donation transaction. I also added a simple view that will place the returned data in in a table (this view was missing, never created, or was meant to be shared with add_donor).

I do need to go back and ensure the query is only returning distinct values. Right now it'd return duplicate values if a user had multiple donation transactions.

UPDATE: /donors now returning only distinct rows. Tested by manually adding transactions to the database, varying number of transactions by user and transaction types. Results are ordered by primary key (by default). We'll let the front-end team decide what they want the returned order to be and update.

@briswells
Copy link
Contributor

Looks like the front end team wants us to bring the math into the backend. I know at least the /charts has a bunch of math in the front end, but be sure to check on any others while converting them.

@MMurtey
Copy link
Contributor

MMurtey commented Mar 30, 2023

Looking through history, it doesn't appear the add_donor route has been converted - I'll do that one real quick. I think logic will be useful one way or another, but the the donor/add_donor/(delete_donor?) route thing seems a bit inefficient. It came up today, so I think we should discuss how the front-end teams wants to communicate with the back-end (I'm betting JSON in/out).

@briswells
Copy link
Contributor

Looking through history, it doesn't appear the add_donor route has been converted - I'll do that one real quick. I think logic will be useful one way or another, but the the donor/add_donor/(delete_donor?) route thing seems a bit inefficient. It came up today, so I think we should discuss how the front-end teams wants to communicate with the back-end (I'm betting JSON in/out).

Not sure when it happened but add_donor was converted, but if you wanted to update the logic go for it. I don't think we'd want to completely delete a person out of the database but we could probably strip out their contact information etc.

We should definitely move to all JSON, luckily for most of the routes all we need to do is update a single line. Maybe while the front end team gets their stuff working as the new frontend container we can put together a Swagger with the current routes? After that's done we can convert what we have to JSON, that way the website still works with the current frontend?

@MMurtey
Copy link
Contributor

MMurtey commented Apr 4, 2023

I just looked at add_donor again- in any case, a donor is only defined by transaction, so it's just creating another user which presents an interesting, if unlikely, scenario: a donor needs to create an account to "work" the pantry. The user already exists, maybe with a dummy password, but without some sort of flag to inform the user signing up what has happened, the best they can do is shrug their shoulders and hit "Forgot Password."

@briswells
Copy link
Contributor

I noticed that the other day, a long with another minor issue with how transactions are processed, I just haven't gotten around to sitting down and fixing it. I don't think it'll be to hard to sort out one way or another.

@MMurtey
Copy link
Contributor

MMurtey commented Apr 6, 2023

Since we're moving the API direction, should we close this issue and associated branch and open a new one? We discussed a bit in person: I think it'd be a good idea to keep the Swagger source in the repo so we can team up on it. We can come up with a way to publish the generated Swagger docs.

@hardikpatil hardikpatil added the backend concerns backend/API presentation label Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend concerns backend/API presentation
Projects
None yet
Development

No branches or pull requests

6 participants