[CR] OSF addon for Microsoft OneDrive#4681
[CR] OSF addon for Microsoft OneDrive#4681caseyrygt wants to merge 59 commits intoCenterForOpenScience:developfrom
Conversation
…ere with wiring up onedrive
Feature/onedrive
…todo:refresh access token and get folder list
Feature/onedrive
…sh and getting auth token
Feature/onedrive
…h folder selection
…ntents out of onedrive
|
@samchrisinger & @felliott: Sorry for the delay on this. I think I have resolved all of the issues / items you had. Could you please review this? Thanks! |
|
@samchrisinger: I meat to ask you about an error I'm getting in the test_views tests: In looking at the dropbox test_views, it calls: self.project.api_url_for. Unfortunately, when I call that method I get: AttributeError: 'TestAuthViews' object has no attribute 'project' I'm guessing that between when I'd copied the tests from dropbox to now, some other changes were made to OSF's testing or "base" framework. Could you give me any points for resolving this? Thanks. |
|
@caseyrygt some of these are happening because the route just doesn't exist: https://github.com/CenterForOpenScience/osf.io/pull/4681/files#diff-0ee0e60ebe1815cd3f65f833c2e4976aR35 I can't find an example test case using 'onedrive_folder_list', but all of the urls you try to look up need to exist in your onedrive/routes.py. |
| 'profile_url': userInfo['link'] | ||
| } | ||
|
|
||
| def _refresh_token(self, access_token, refresh_token): |
There was a problem hiding this comment.
This and the two other refresh-related functions in this class are likely superfluous, with auto_refresh_url defined. See website/oauth/models/__init__.py:ExternalProvider.refresh_oauth_key -- it does all of this work.
This would also obviate OneDriveAuthClient.refresh; .user_info could be moved into OneDriveClient.
|
Pausing review here for the night, picking up at the serializer in the AM. |
|
|
||
|
|
||
| class OneDriveSerializer(OAuthAddonSerializer): | ||
| def credentials_owner(self, user_settings=None): |
There was a problem hiding this comment.
This should probably be using the base implementation. Currently it would throw an AttributeError if user_settings isn't passed in.
|
Trivial, but all the console logging will need to go before this gets merged. |
|
Somewhat light first pass complete. 🐧 I'll review in more depth and do functional testing when the above comments have been addressed. I know updating the views and tests to the new paradigm seems like a lot of work, but there's a good chance it'll be simpler for both of us than fixing and verifying the correctness of all the commented-out tests that presumably need some amount of maintenance. If you have any questions or need any clarifications, feel free to @ mention me, or if you're on Slack I can hop on and discuss things with you there. Back to you, @caseyrygt |
Initial Work In Progress PR for Microsoft OneDrive OSF addon
Notes
We copied Box as the starting point for this addon. There are debug and info logging statements that we'll remove before the final PR.
Also see the corresponding PR for WaterButler.
Limitations
Issues under development
Functionality in Place (i.e. "dev complete")
Screenshots
Note: I don't have MFR running. I'm having issues getting it running in ubuntu.