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(Google Drive Node): Fix regex in file RLC #6607

Merged
merged 5 commits into from
Jul 5, 2023

Conversation

michael-radency
Copy link
Contributor

Github issue / Community forum post (link here to close automatically):

@michael-radency michael-radency added n8n team Authored by the n8n team node/issue Issue with a node labels Jul 5, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jul 5, 2023

Great PR! Please pay attention to the following items before merging:

Files matching packages/**:

  • If fixing bug, added test to cover scenario.
  • If addressing forum or Github issue, added link to description.

Files matching packages/**/*.ts:

  • Added unit tests to cover new or updated functionality.

Files matching packages/nodes-base/nodes/**:

  • Added workflow tests for nodes if possible.

Make sure to check off this list before asking for review.

Joffcom
Joffcom previously approved these changes Jul 5, 2023
Copy link
Member

@Joffcom Joffcom left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@netroy netroy left a comment

Choose a reason for hiding this comment

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

don't we need to update the v1 node as well ?

@cypress
Copy link

cypress bot commented Jul 5, 2023

Passing run #1391 ↗︎

0 228 0 0 Flakiness 0

Details:

🌳 node-128-google-drive-node-overhaul-rlc-fix 🖥️ browsers:node18.12.0-chrome10...
Project: n8n Commit: 4d333f1daf
Status: Passed Duration: 07:05 💡
Started: Jul 5, 2023 8:07 PM Ended: Jul 5, 2023 8:14 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 5, 2023

✅ All Cypress E2E specs passed

Copy link
Member

@netroy netroy left a comment

Choose a reason for hiding this comment

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

While we are doing this, we should copy all of the 12 variants on this regexp string into a new file packages/nodes-base/nodes/Google/constants.ts as a single constant, instead of repeating this so many times.
This way we won't end up with multiple variants of this regexp string again.

@codecov
Copy link

codecov bot commented Jul 5, 2023

Codecov Report

Patch coverage: 37.50% and project coverage change: -0.07 ⚠️

Comparison is base (8abb03d) 28.91% compared to head (4d333f1) 28.84%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6607      +/-   ##
==========================================
- Coverage   28.91%   28.84%   -0.07%     
==========================================
  Files        3070     3071       +1     
  Lines      188321   188329       +8     
  Branches    20887    20884       -3     
==========================================
- Hits        54454    54326     -128     
- Misses     132983   133119     +136     
  Partials      884      884              
Impacted Files Coverage Δ
...base/nodes/Google/Drive/GoogleDriveTrigger.node.ts 0.00% <0.00%> (ø)
...s-base/nodes/Google/Drive/v1/GoogleDriveV1.node.ts 0.00% <0.00%> (ø)
...ase/nodes/Google/Sheet/GoogleSheetsTrigger.node.ts 0.00% <0.00%> (ø)
...es/Google/Sheet/v2/actions/sheet/Sheet.resource.ts 0.00% <0.00%> (ø)
...e/Sheet/v2/actions/spreadsheet/delete.operation.ts 0.00% <0.00%> (ø)
...des/Google/Drive/v2/actions/common.descriptions.ts 100.00% <100.00%> (ø)
packages/nodes-base/nodes/Google/constants.ts 100.00% <100.00%> (ø)

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@michael-radency
Copy link
Contributor Author

While we are doing this, we should copy all of the 12 variants on this regexp string into a new file packages/nodes-base/nodes/Google/constants.ts as a single constant, instead of repeating this so many times. This way we won't end up with multiple variants of this regexp string again.

thanks for suggestion @netroy, pushed update

Copy link
Member

@netroy netroy left a comment

Choose a reason for hiding this comment

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

LGTM

@Joffcom
Copy link
Member

Joffcom commented Jul 5, 2023

That was a good spot 🙌

Copy link
Member

@Joffcom Joffcom left a comment

Choose a reason for hiding this comment

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

Approving again just to be complete 🙏

@github-actions
Copy link
Contributor

github-actions bot commented Jul 5, 2023

✅ All Cypress E2E specs passed

1 similar comment
@github-actions
Copy link
Contributor

github-actions bot commented Jul 5, 2023

✅ All Cypress E2E specs passed

@michael-radency michael-radency merged commit 5672146 into master Jul 5, 2023
@michael-radency michael-radency deleted the node-128-google-drive-node-overhaul-rlc-fix branch July 5, 2023 20:15
MiloradFilipovic added a commit that referenced this pull request Jul 6, 2023
* master: (50 commits)
  fix(Google Drive Node): Fix regex in file RLC (#6607)
  feat(HTML Node): 'Convert to table operation (#6540)
  ci: e2e tests don't need to enable saving manual executions
  🚀 Release 1.0.1 (#6606)
  fix(core): Make node execution order configurable, and backward-compatible (#6507)
  🚀 Release 0.236.0 (#6602)
  fix: Fix migrations for postgres and mysql (no-changelog) (#6600)
  fix(core): Improve the performance of last 2 sqlite migrations (#6522)
  refactor: Switch cloud plan retrieval endpoint (no-changelog) (#6529)
  fix(Salesforce Node): Fix typo for adding a contact to a campaign (#6598)
  ci: Hide v1 banner in e2e tests to fix 2-credentials.cy.ts (no-changelog)
  fix(editor): Restore expression completions (#6566)
  refactor: Remove basic auth env vars from docker compose stacks (#6570)
  fix(core): Fix migrations for MySQL/MariaDB (#6591)
  fix(editor): Show retry information in execution list only when it exists (#6587)
  fix(Strapi Node): Fix issue with pagination (#4991)
  fix(core): Route `/rest/workflows/new` correctly (no-changelog) (#6572)
  fix(XML Node): Fix issue with not returning valid data (#6565)
  fix(core): Fix credentials test (#6569)
  fix(core): Ensure valid `logger` is passed to every migration (no-changelog) (#6563)
  ...

# Conflicts:
#	packages/design-system/src/components/N8nCallout/Callout.vue
#	packages/editor-ui/src/App.vue
#	packages/editor-ui/src/plugins/i18n/locales/en.json
#	packages/editor-ui/src/stores/ui.store.ts
MiloradFilipovic added a commit that referenced this pull request Jul 7, 2023
* ADO-814-trial-banner: (58 commits)
  ⚡ Updating permanent dismiss logic to work with all banners
  fix(editor): Make Source control branch select required (#6619)
  ⚡ Minor refactoring of banner logic
  refactor(core): Load `cookieParser` middleware only once (no-changelog) (#6614)
  fix(editor): Prevent keyboard shortcuts to edit workflows in readonly mode (#6613)
  fix(editor): Show appropriate empty workflow list content when instance environment is readonly (#6610)
  ✨ Implemented permanent/temporary dismiss logic for v1 flag
  ci: Fix linting issues (no-changelog)
  fix(Google Drive Node): Fix regex in file RLC (#6607)
  feat(HTML Node): 'Convert to table operation (#6540)
  ci: e2e tests don't need to enable saving manual executions
  🚀 Release 1.0.1 (#6606)
  fix(core): Make node execution order configurable, and backward-compatible (#6507)
  🚀 Release 0.236.0 (#6602)
  fix: Fix migrations for postgres and mysql (no-changelog) (#6600)
  fix(core): Improve the performance of last 2 sqlite migrations (#6522)
  refactor: Switch cloud plan retrieval endpoint (no-changelog) (#6529)
  fix(Salesforce Node): Fix typo for adding a contact to a campaign (#6598)
  ci: Hide v1 banner in e2e tests to fix 2-credentials.cy.ts (no-changelog)
  fix(editor): Restore expression completions (#6566)
  ...
@github-actions github-actions bot mentioned this pull request Jul 12, 2023
@janober
Copy link
Member

janober commented Jul 12, 2023

Got released with [email protected]

@github-actions github-actions bot mentioned this pull request Jul 12, 2023
@github-actions github-actions bot mentioned this pull request Jul 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
n8n team Authored by the n8n team node/issue Issue with a node Released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants