Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

feat: import google sheet dataset #14

Merged
merged 29 commits into from
Jul 8, 2021
Merged

feat: import google sheet dataset #14

merged 29 commits into from
Jul 8, 2021

Conversation

issmail-basel
Copy link
Contributor

@issmail-basel issmail-basel commented Jun 13, 2021 •
edited
Loading

  • Add gapi (google API) script when initializing the component
  • Initializing both (auth and file picker)
  • Put the right permissions
  • Add required modals
  • Call backend mutation for importing datasheets
    Screenshots

1
2
3
4
2021-06-23_13-32

@codecov
Copy link

codecov bot commented Jun 13, 2021 •
edited
Loading

Codecov Report

Merging #14 (210921e) into main (51de778) will decrease coverage by 0.08%.
The diff coverage is n/a.

❗ Current head 210921e differs from pull request most recent head 00bccf4. Consider uploading reports for the commit 00bccf4 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main      #14      +/-   ##
==========================================
- Coverage   52.32%   52.24%   -0.09%     
==========================================
  Files          45       45              
  Lines         644      645       +1     
  Branches      125      125              
==========================================
  Hits          337      337              
- Misses        232      233       +1     
  Partials       75       75
Impacted Files Coverage Δ
src/gql/graphql-client-api.tsx 52.63% <ø> (-0.03%) ⬇️
src/components/atoms/Markdown/index.tsx 54.54% <0.00%> (-1.98%) ⬇️
src/util/arrayDiff.ts 100.00% <0.00%> (ø)

@issmail-basel issmail-basel changed the title Feat: import google sheet dataset feat: import google sheet dataset Jun 21, 2021
@issmail-basel issmail-basel marked this pull request as ready for review June 21, 2021 21:07
KaWaite
KaWaite reviewed Jun 23, 2021
View reviewed changes
Copy link
Member

@KaWaite KaWaite left a comment

Choose a reason for hiding this comment

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

Left some comments. Let me know if you have any questions or when you are ready for my re-review!

KaWaite
KaWaite previously approved these changes Jun 23, 2021
View reviewed changes
Copy link
Member

@KaWaite KaWaite left a comment

Choose a reason for hiding this comment

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

Please change where I made a comment about English.
But other than that looks good to me!

HideBa
HideBa previously approved these changes Jul 2, 2021
View reviewed changes
Copy link
Member

@HideBa HideBa left a comment

Choose a reason for hiding this comment

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

This PR looks good to me. 👍 However, the AssetModal had been changed somehow. Asset and Dataset is a different thing for Re:Earth. As a refactoring task, AssetModal can be renamed GridModal or CardModal and made a generic component which accepts Asset type and Dataset type.

src/components/molecules/EarthEditor/DatasetPane/DatasetModal/Gdrive/hooks.ts Outdated Show resolved Hide resolved
KaWaite
KaWaite reviewed Jul 2, 2021
View reviewed changes
Copy link
Member

@KaWaite KaWaite left a comment

Choose a reason for hiding this comment

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

Sorry looks like you mixed current code with old code when you merged. Please check the comment I left

rot1024
rot1024 suggested changes Jul 5, 2021
View reviewed changes
Copy link
Member

@rot1024 rot1024 left a comment

Choose a reason for hiding this comment

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

Have you considered using https://www.npmjs.com/package/react-google-picker ?

src/components/molecules/EarthEditor/DatasetPane/DatasetModal/hooks.ts Outdated Show resolved Hide resolved
src/components/molecules/EarthEditor/DatasetPane/DatasetModal/Gdrive/hooks.ts Outdated Show resolved Hide resolved
src/components/molecules/EarthEditor/DatasetPane/DatasetModal/index.tsx Outdated Show resolved Hide resolved
@issmail-basel
Copy link
Contributor Author

@rot1024 , yes I've considered using https://www.npmjs.com/package/react-google-picker
but we only use this picker once, and in a very simple way, so I guess no need for an external package.
If there is any future need, I'll consider changing my code

rot1024
rot1024 previously approved these changes Jul 8, 2021
View reviewed changes
HideBa
HideBa previously approved these changes Jul 8, 2021
View reviewed changes
@issmail-basel issmail-basel dismissed stale reviews from HideBa and rot1024 via 003851a July 8, 2021 07:50
@issmail-basel issmail-basel merged commit 21b167f into main Jul 8, 2021
@issmail-basel issmail-basel deleted the feat/import-google-sheet branch July 8, 2021 09:06
keiya01 pushed a commit that referenced this pull request Apr 25, 2023
@mimoham24
fix updateme payload (#14)
ef75dd0
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lavalse lavalse lavalse left review comments

@rot1024 rot1024 rot1024 left review comments

@HideBa HideBa HideBa left review comments

@KaWaite KaWaite Awaiting requested review from KaWaite KaWaite is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@issmail-basel @lavalse @rot1024 @KaWaite @HideBa @Basel-Issmail