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

fix!: switch google drive picker to gis #23096

Merged
merged 18 commits into from
Nov 29, 2023

Conversation

TMF42
Copy link
Contributor

@TMF42 TMF42 commented Nov 4, 2023

Important: People migrating from the old setup will have to generate a new OAuth Client Key as well as a new API key.

Original PR to integrate Google Drive Picker in file uploader -> #12715
Created by @barredterra

Meanwhile, Google deprecated the Google Sign-In library and new client IDs will be blocked -> see here

This PR deals with migrating the authentication part to Google Identity Services based on Google's suggestions -> see here
and based on the latest example for using the Google Picker API -> see here

Additional Notes:

  • no UI changes

@TMF42 TMF42 requested review from a team and shariquerik and removed request for a team November 4, 2023 12:37
Copy link
Collaborator

@barredterra barredterra left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the fix! Apart from the suggestions above, this seems to work fine.

People migrating from the old setup will have to generate a new OAuth Client Key as well as a new API key.

@barredterra barredterra added backport version-14-hotfix backport to version 14 backport version-15-hotfix Backport the PR to v15 labels Nov 5, 2023
TMF42 and others added 3 commits November 6, 2023 08:59
Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>
Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>
Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>
@barredterra barredterra self-assigned this Nov 7, 2023
@barredterra
Copy link
Collaborator

@TMF42 sorry for the delay, I'm pretty busy at the moment.

If anybody else can review this, please do!

@barredterra barredterra removed their assignment Nov 21, 2023
@fadilsiddique
Copy link
Contributor

@TMF42 will this fix the authentication error currently facing in the google drive picker?

reference: https://discuss.frappe.io/t/google-drive-picker/102950

@TMF42
Copy link
Contributor Author

TMF42 commented Nov 24, 2023

@TMF42 will this fix the authentication error currently facing in the google drive picker?

reference: https://discuss.frappe.io/t/google-drive-picker/102950

@fadilsiddique yes. that's the fix for the problem you referenced.

@fadilsid
Copy link
Contributor

@TMF42 will this fix the authentication error currently facing in the google drive picker?

reference: https://discuss.frappe.io/t/google-drive-picker/102950

@fadilsiddique yes. that's the fix for the problem you referenced.

Thanks. Facing the issue currently. I have asked in the TG group to check this PR.

Copy link
Collaborator

@barredterra barredterra left a comment

Choose a reason for hiding this comment

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

I'm not very happy with the code. Could definitely use a refactor. But it works fine, so I'm inclined to get this merged for the time being.

Todo:

  • Remove unused properties (this.pickerInited, this.gisInited)
  • Replace this.accessToken with frappe.boot.user.google_drive_token. No need to store this in two places. No need to store this at all.
  • Currently, the auth popup shows every time. Can we avoid this, if we already have an active token?

@barredterra barredterra enabled auto-merge (squash) November 25, 2023 17:50
@barredterra barredterra changed the title fix: switch google drive picker to gis fix!: switch google drive picker to gis Nov 25, 2023
@barredterra
Copy link
Collaborator

@shariquerik this PR is ready. I have tested the functionality. If the code looks okay, kindly merge.

@ankush ankush disabled auto-merge November 29, 2023 18:09
@ankush ankush merged commit 043c004 into frappe:develop Nov 29, 2023
19 of 23 checks passed
mergify bot pushed a commit that referenced this pull request Nov 29, 2023
* fix: switch google drive picker to gis

* fix: switch google drive picker to gis (linting)

* fix: switch google drive picker to gis (prettier)

* fix: remove restrictions on file types

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>

* fix: show navigator

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>

* fix: no multiselect for files

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>

* refactor: get rid of jquery

* fix: don't add script twice

* refactor: remove unused properties

* refactor: store access token in one place only

* refactor: make tokenClient a local constant

* fix: set locale

* refactor: async calls

* fix: don't store access token

It's one-time use anyway.

* fix: scope for allowing file upload

* reafctor: rename libsLoaded() to autthenticate()

---------

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>
(cherry picked from commit 043c004)
mergify bot pushed a commit that referenced this pull request Nov 29, 2023
* fix: switch google drive picker to gis

* fix: switch google drive picker to gis (linting)

* fix: switch google drive picker to gis (prettier)

* fix: remove restrictions on file types

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>

* fix: show navigator

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>

* fix: no multiselect for files

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>

* refactor: get rid of jquery

* fix: don't add script twice

* refactor: remove unused properties

* refactor: store access token in one place only

* refactor: make tokenClient a local constant

* fix: set locale

* refactor: async calls

* fix: don't store access token

It's one-time use anyway.

* fix: scope for allowing file upload

* reafctor: rename libsLoaded() to autthenticate()

---------

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>
(cherry picked from commit 043c004)
ankush pushed a commit that referenced this pull request Nov 29, 2023
* fix: switch google drive picker to gis

* fix: switch google drive picker to gis (linting)

* fix: switch google drive picker to gis (prettier)

* fix: remove restrictions on file types

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>

* fix: show navigator

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>

* fix: no multiselect for files

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>

* refactor: get rid of jquery

* fix: don't add script twice

* refactor: remove unused properties

* refactor: store access token in one place only

* refactor: make tokenClient a local constant

* fix: set locale

* refactor: async calls

* fix: don't store access token

It's one-time use anyway.

* fix: scope for allowing file upload

* reafctor: rename libsLoaded() to autthenticate()

---------

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>
(cherry picked from commit 043c004)

Co-authored-by: Thomas Fojan <thomas@tmf.one>
ankush pushed a commit that referenced this pull request Nov 29, 2023
* fix: switch google drive picker to gis

* fix: switch google drive picker to gis (linting)

* fix: switch google drive picker to gis (prettier)

* fix: remove restrictions on file types

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>

* fix: show navigator

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>

* fix: no multiselect for files

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>

* refactor: get rid of jquery

* fix: don't add script twice

* refactor: remove unused properties

* refactor: store access token in one place only

* refactor: make tokenClient a local constant

* fix: set locale

* refactor: async calls

* fix: don't store access token

It's one-time use anyway.

* fix: scope for allowing file upload

* reafctor: rename libsLoaded() to autthenticate()

---------

Co-authored-by: Raffael Meyer <14891507+barredterra@users.noreply.github.com>
(cherry picked from commit 043c004)

Co-authored-by: Thomas Fojan <thomas@tmf.one>
@TMF42 TMF42 deleted the fix/google-drive-picker branch November 30, 2023 07:00
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport version-14-hotfix backport to version 14 backport version-15-hotfix Backport the PR to v15 squash
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants