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 batchwrite functionality #144

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

abkarino
Copy link

@abkarino abkarino commented Oct 5, 2022

Resolves #65 , resolves #54
This allows batchwrites which has a solution for URL limit since the mask is put in the body.
I did forget to remove one line in the package.json file that changed the repo param and will need to be reverted back before merging.

Checklist for your code

  • Have you all the appropriate dependencies (after git clone)?
    • Install all packages from package.json with a bare npm install.
  • [-] Have you tested the code yourself? (I published it as a library and linked it in my main branch)
    • Install clasp npm install --global @google/clasp
    • Log into your Google Account clasp login
    • Create/Clone Apps Script Project to link and set file extension to "ts". Example .clasp.json file:
      {
        "scriptId": "1yIYw-your-scriptID-here-las12jsA2sd",
        "fileExtension": "ts"
      }
    • Add storable credentials to your script to connect to your Firestore database.
    • Add test methods to Tests.ts file which validates your change. GSUnit Reference (this one is missing)
    • Debug/Test your code against your database.
  • Have you documented your functions and their parameters and return values with JSDoc? (I documented the function overloading like how it was documented on Google Firebase JS SDK but it seems that it doesn't get reflected when converted to JS, I didn't update the readme file yet)
  • Have you modernized the code to run with the new V8 Runtime?
  • Have you privatized or encapsulated all intended functions in a class?
    • Global functions can be privatized by appending the "_" to the function name.
  • Have you ensured all the appropriate Typescript Definitions are available?
  • Does your code follow the Prettier Javascript Style?
    • Check syntax and style in one go with npm run lint.
    • Autofix most issues with npm run fix.

Copy link
Collaborator

@LaughDonor LaughDonor left a comment

Choose a reason for hiding this comment

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

Good catch on updating the typings!

You're missing the key updates in Tests.ts and README with how to utilize this feature, please update these as well. Thanks for the awesome work so far!

Firestore.ts Show resolved Hide resolved
WriteBatch.ts Outdated Show resolved Hide resolved
WriteBatch.ts Outdated Show resolved Hide resolved
WriteBatch.ts Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
typings/gapi.client.firestore-v1/index.d.ts Show resolved Hide resolved
@LaughDonor
Copy link
Collaborator

I need to test these via clasp upload before I can accept. I plan to do this in the coming days.

@ciriousjoker
Copy link

Any updates?

@trettimann
Copy link

trettimann commented Nov 7, 2022

@LaughDonor Did you get around to testing it? :)

@abkarino
Copy link
Author

abkarino commented Nov 8, 2022

The readme is updated, only writing tests remain.

@leonardorb
Copy link

hey folks, are we planning to promote this to the library soon?

@ciriousjoker
Copy link

@abkarino @grahamearley Will this be merged at some point?

@abkarino
Copy link
Author

abkarino commented Jun 3, 2023

@abkarino @grahamearley Will this be merged at some point?

I am afraid, it is not really in my hand. At the moment, it only needs the tests to be written and the project maintainers to test it, verify it and merge it. I am already using it in my repo with some work projects.

@charan-vendra
Copy link

Hi @LaughDonor,
Is this code going to be merged soon?

@abkarino great work, could you request a re-review?

@MichaelJoo
Copy link

I am really looking forward for this update to be merged - I have hundreds of sheets to import into Firestore, and each sheet is going to take at least 30 minutes or 1 hour to be fully imported. I reckon adding this feature would definitely reduce the time spent running the script.

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.

Feature request: batched writes Limit Exceeded: URLFetch URL Length. in updateDocument with mask=true
7 participants