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

Update app to use a single endpoint for google and non-google stored brews #2089

Merged
merged 7 commits into from
Mar 27, 2022

Conversation

jeddai
Copy link
Collaborator

@jeddai jeddai commented Mar 24, 2022

I'm thinking maybe the Homebrew API methods that are LocalBrew and GoogleBrew might could be moved out of the api file and put in their own file, but that's really the only thing I'm still thinking about. Otherwise I think this is good to go!

See #2019

@calculuschild
Copy link
Member

calculuschild commented Mar 24, 2022

Ok, so.... this all looks to be in order? 🎉 I haven't tested it yet but all your changes make sense. That's a lot of logic to shuffle around so I applaud your work. Just those couple small suggestions above and I'll test it a bit on my own, but looks pretty good to me!

Also, make sure your NPM version is updated, and the dependencies are up-to-date as well. The package-lock.json file in this PR has changed to "version 1 lockfile" which is older. Actually, can you remove the changes to package-lock.json from this PR altogether?

@jeddai
Copy link
Collaborator Author

jeddai commented Mar 24, 2022

Switched my node version to 16.14.0, seems to use the package-lock v2 now. Should be good for testing!

@G-Ambatte
Copy link
Collaborator

Unifying brew handling to a single interface should simplify the basePage editorPage PR (#1800/#1801) as well - handling saving was one of the points of significant complexity.

@calculuschild
Copy link
Member

This all seems to work after testing. I'll go ahead and merge this. 🤞

One small edge case that I'll make a separate issue for: since this keeps the same IDs between Google Drive/ Homebrewery when transferring the file, it's possible to end up with a duplicate ID error from Mongo if you transfer from Google (Google Brew lands in the Trash), then visit the old link to that trashed file/restore the trashed file so it shows up in your user page, attempt to transfer that file back to the Homebrewery as well. The error however manifests as "your Google Credentials are expired" popup. I am doubtful it will ever come up but thought I'd document it.

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