Skip to content

Conversation

@jorwoods
Copy link
Contributor

Solves #39

This may not be the intended implementation, or be how it was intended to be solved. I took a pass at it to give myself a route to modify favorites for my users. After I wrote this, I noticed stubs for it in the users endpoint, but wasn't sure if that was still the plan as favorites has a separate base url. I haven't written extensive tests for this yet.

Also wasn't sure if "Favorites" technically should have their own model, or just an endpoint. Sort of ambiguous as a favorites object would just be a wrapper around a workbook/view/project/datasource.

Copy link
Collaborator

@t8y8 t8y8 left a comment

Choose a reason for hiding this comment

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

@jorwoods thanks for the PR!

I left a few comments, and you should add a test or two as well (as you already noted)

@jorwoods
Copy link
Contributor Author

Added testing, and made the favorites load in as a dictionary attached to the user item.

@jorwoods jorwoods force-pushed the jorwoods/user_favorites branch from e364a9c to 40cd444 Compare May 28, 2020 17:34
@jorwoods jorwoods force-pushed the jorwoods/user_favorites branch from 581088b to 29768db Compare May 28, 2020 17:54
@jorwoods jorwoods force-pushed the jorwoods/user_favorites branch from 2008932 to 301c257 Compare May 28, 2020 18:12
@jorwoods
Copy link
Contributor Author

jorwoods commented May 28, 2020

Rebased on current development branch

Fixed code style issues
Readded Favorites import
Removed deprecated calls from unrelated tests

@t8y8 does this look ready to merge?

@jorwoods
Copy link
Contributor Author

@t8y8 Anything else need to change?

@jorwoods jorwoods closed this Jun 25, 2020
@jorwoods jorwoods mentioned this pull request Jun 25, 2020
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.

2 participants