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

Add Google file link to UserPage BrewItem #2969

Merged
merged 4 commits into from
Aug 10, 2023

Conversation

G-Ambatte
Copy link
Collaborator

This PR follows up on a side issue raised in #2954.

This PR adds a link to the file on Google Drive via the Drive Icon on the User Page. In the event that a user cannot find their brew document but it does still exist in their Drive somewhere, they can use this link to locate the file.

@G-Ambatte G-Ambatte requested a review from calculuschild August 7, 2023 10:07
@G-Ambatte G-Ambatte self-assigned this Aug 7, 2023
@G-Ambatte G-Ambatte added the P2 - minor feature or not urgent Minor bugs or less-popular features label Aug 7, 2023
@G-Ambatte G-Ambatte changed the title Add google file link Add Google file link to UserPage BrewItem Aug 7, 2023
@G-Ambatte
Copy link
Collaborator Author

The second part of this PR adds a small icon for brews stored on the local MongoDB database, to differentiate them from the brews stored on Google Drive.


return <span>
<img className='googleDriveIcon' src={googleDriveIcon} alt='googleDriveIcon' />
return <span title='Google Drive Storage'>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe the Title should be “Saved on [User]’s Google Drive”?

Copy link
Member

@calculuschild calculuschild Aug 9, 2023

Choose a reason for hiding this comment

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

This could be done with the owners field from the Google API. Unfortunately it would only work for brews owned by the current logged-in user where we call listGoogleBrews()

Maybe in that case we could display Another User's Google Drive Storage to at least make clear that you aren't logged in to the owning account.

@ericscheid
Copy link
Collaborator

Does webViewLink work if the user looking at the Users Page is not the owner of that User Page?

For example, if anyone but me looks at https://homebrewery.naturalcrit.com/user/erics, will the Google Drive link for the one brew I have there on gdrive work for them?

@G-Ambatte
Copy link
Collaborator Author

will the Google Drive link for the one brew I have there on gdrive work for them?

A quick perusal of the code shows that when ownAccount is false, GoogleActions.listGoogleBrews() is not performed, so webViewLink cannot be retrieved from Google Drive. The information about the listed brews that are stored on Google Drive comes from the internal Brew stub.
webViewLink has not been added to the fields array, so even if it does get stored in the Brew stub, it will not be retrieved.

When webViewLink is undefined, the resulting <a> element has no href, and looks like this:

image

Copy link
Member

@calculuschild calculuschild left a comment

Choose a reason for hiding this comment

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

Ok, so as pointed out, this only works if you are logged in to the owner's google drive account. If not, the webViewLink is undefined since we are just reading from the stub without actually performing listGoogleBrews(); Often the reason a user can't find the file is because they aren't logged in to the right account. So what should we display to the user in that case?

Edit: It looks like the webViewLink can by derived by just using the Google ID which we have, so we could apply this to all brews on the page, even those from a Stub:

Then the link would work for everyone, even if logged out (or in the wrong account). Displaying whether that file exists is another question, but visiting the link would at least show an error from google in that case:

image

@G-Ambatte
Copy link
Collaborator Author

Often the reason a user can't find the file is because they aren't logged in to the right account. So what should we display to the user in that case?

Edit: It looks like the webViewLink can by derived by just using the Google ID which we have, so we could apply this to all brews on the page, even those from a Stub:

* **googleID** : 1eynUmZHfmTc6fDQupTvxgxlhxGyrwoPw

* **webViewLink** : https://drive.google.com/file/d/1eynUmZHfmTc6fDQupTvxgxlhxGyrwoPw/view?usp=drivesdk

Then the link would work for everyone, even if logged out (or in the wrong account).

Given that every Google brew needs to be set as 'Anyone with the link can Edit' in order for things to work, exposing the link is problematic, from a data security perspective.
My personal tendency would be towards providing the link on the owner's account, "Another user's Google Drive" for non-owners of Google Drive docs, and "Homebrewery Storage" for internal brews.

@calculuschild
Copy link
Member

Given that every Google brew needs to be set as 'Anyone with the link can Edit' in order for things to work, exposing the link is problematic, from a data security perspective.

Hmm, good point. I was thinking only about users who are looking at their own accounts, meaning they are already also authors and should have edit privileges anyway, but this would expose the link to any random visitor to your user page too.

What about showing the link only if you are looking at your own brews page? I think there may still be some benefit to being able to link to files you are an author on, even if you don't own it. For the same reasons as before (user has multiple Google accounts and has "lost" the file, etc.)?

@G-Ambatte
Copy link
Collaborator Author

What about showing the link only if you are looking at your own brews page?

That seems reasonable, I think - if you're an author, you can edit the brew, so being able to see the source file poses a minimal additional security exposure. The only negative case I can think of immediately is a removed author might be able to retain access via the Google Drive link in their history. The upside to that is that Google Drive version history allows for the relatively painless restoration of the file in the event that a malicious ex-author vandalizes a file.

@G-Ambatte
Copy link
Collaborator Author

G-Ambatte commented Aug 10, 2023

Also, the presence of a this.props.webViewLink implies the current user's account is the owner (in that GoogleActions.listGoogleBrew only occurs if the user account is the owner), so it might be a better property to test than authors[0].

@calculuschild
Copy link
Member

Ok, I think this is good enough as is. I'll go ahead and merge. Thanks @G-Ambatte !

@calculuschild calculuschild merged commit bac5372 into naturalcrit:master Aug 10, 2023
@G-Ambatte G-Ambatte deleted the addGoogleFileLink branch August 11, 2023 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 - minor feature or not urgent Minor bugs or less-popular features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants