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

feat: Add dangerouslyUseHTMLString where needed (no-changelog) #6292

Merged
merged 1 commit into from
May 22, 2023

Conversation

alexgrozav
Copy link
Contributor

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

@github-actions
Copy link
Contributor

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 **/*.vue:

  • Used composition API for all new components.
  • Added component or unit tests to cover functionality.

Files matching packages/editor-ui/**/*.vue:

  • Added E2E if adding new features.
  • Used design system tokens (colors, spacings...) where possible.

Files matching packages/editor-ui/src/mixins/**:

  • Avoided adding new mixins (use composables instead). Only removed code from here.

Files matching packages/editor-ui/src/views/NodeView.vue:

  • Avoided adding code here. Only refactored to make it smaller.

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

@alexgrozav alexgrozav changed the title feat: add dangerouslyUseHTMLString where needed (no-changelog) feat: Add dangerouslyUseHTMLString where needed (no-changelog) May 22, 2023
@n8n-assistant n8n-assistant bot added n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system labels May 22, 2023
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.

Do you know if it would possible to detect html like this during build (using a custom vite plugin), and fail the build if dangerouslyUseHTMLString is not set?
otherwise this issue is likely to back every now and then.

@github-actions
Copy link
Contributor

✅ All Cypress E2E specs passed

@codecov
Copy link

codecov bot commented May 22, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.01 ⚠️

Comparison is base (2d13b3f) 27.65% compared to head (e75854b) 27.65%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6292      +/-   ##
==========================================
- Coverage   27.65%   27.65%   -0.01%     
==========================================
  Files        2948     2948              
  Lines      180860   180872      +12     
  Branches    19642    19642              
==========================================
  Hits        50019    50019              
- Misses     130094   130106      +12     
  Partials      747      747              
Impacted Files Coverage Δ
packages/editor-ui/src/App.vue 0.00% <0.00%> (ø)
...es/editor-ui/src/components/ParameterInputFull.vue 43.85% <0.00%> (-0.13%) ⬇️
...ges/editor-ui/src/components/WorkflowActivator.vue 58.92% <0.00%> (-0.36%) ⬇️
...editor-ui/src/components/WorkflowShareModal.ee.vue 0.00% <0.00%> (ø)
packages/editor-ui/src/composables/useToast.ts 26.19% <0.00%> (-0.32%) ⬇️
packages/editor-ui/src/mixins/genericHelpers.ts 58.24% <0.00%> (-0.65%) ⬇️
packages/editor-ui/src/mixins/pushConnection.ts 14.42% <0.00%> (-0.03%) ⬇️
packages/editor-ui/src/mixins/workflowHelpers.ts 13.82% <0.00%> (-0.02%) ⬇️
packages/editor-ui/src/views/NodeView.vue 13.49% <0.00%> (-0.01%) ⬇️
packages/editor-ui/src/views/SetupView.vue 17.41% <0.00%> (-0.09%) ⬇️

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

@alexgrozav
Copy link
Contributor Author

alexgrozav commented May 22, 2023

dangerouslyUseHTMLString

It's probably possible to implement a custom linting rule. What do you think, @ivov ?

Technically it shouldn't happen. If the dev sees that HTML is rendered as sanitized, they'll be forced to add dangerouslyUseHTMLString, but I guess it can slip.

@alexgrozav alexgrozav requested a review from cstuncsik May 22, 2023 12:47
@github-actions
Copy link
Contributor

✅ All Cypress E2E specs passed

@alexgrozav alexgrozav merged commit 2d90125 into master May 22, 2023
@alexgrozav alexgrozav deleted the dangerously-use-html-string branch May 22, 2023 13:09
@ivov
Copy link
Contributor

ivov commented May 22, 2023

It's probably possible to implement a custom linting rule. What do you think, @ivov ?

Definitely. I haven't written any Vue lint rules yet but I'd be happy to.

eyaljoyous pushed a commit to eyaljoyous/n8n that referenced this pull request May 23, 2023
* allow execution on  node 19,20,22

* fix(ERPNext Node): Fix issue with credential test and add frappe cloud url (n8n-io#6283)

* fix(editor): UI copy fix for Date & Time node (no-changelog) (n8n-io#6291)

* whether -> when

* lint fix

---------

Co-authored-by: Jonathan Bennetts <jonathan.bennetts@gmail.com>

* feat(RabbitMQ Node): Add mode for acknowledging and deleting from queue later in workflow (n8n-io#6225)

* Add later in workflow mode

* Add new operation

* Acknowledge message in next node

* Add response and emit for responsePromiseHook

* Remove double success message, close channel correctly

* Answser messages correctly

* Remove option from delete operation

* move operation name to camelCase

* Fix versioning

* To remove: add action item in v1

* Add notice for delete from queue

* Correctly only execute only the delete operation

* Refactor delete from queue operator and add return last items

---------

Co-authored-by: Marcus <marcus@n8n.io>

* feat: Add dangerouslyUseHTMLString where needed (no-changelog) (n8n-io#6292)

feat: add dangerouslyUseHTMLString where needed (no-changelog)

* fix(Wekan Node): Handle response correctly (n8n-io#6296)

Fix bug when response wasn't array

* Added procfile

* Added procfile

* redployed

* redployed

* redployed

* redployed

* redployed

* redployed

* dont start

* redployed

* redployed

* update version

* redployed

* redployed

* redployed

* redployed

* Revert "update version"

This reverts commit c249585.

* add heroku stack version

* delete

* add app.json setting heroku build stack

* update

* update vite

* update mem

* update PORT

* redployed

* fix(core): Optimize SharedWorkflow queries (n8n-io#6297)

* optimize SharedWorkflow queries

* fix int to string ids

* fix(Strapi Node): Strapi credentials notice (n8n-io#6289)

* fix(SSH Node): Private key field as password and credential test (n8n-io#6298)

* fix: Prevent removing manual executions when setting says to save (n8n-io#6300)

* fix: Initialize license in queue mode correctly (n8n-io#6301)

* feat(Ldap Node): Add LDAP node (n8n-io#4783)

* feat(LoneScale Node): Add LoneScale node and Trigger node (n8n-io#5146)

* feat: Add SSO SAML metadataUrl support and various improvements (n8n-io#6139)

* feat: add various sso improvements

* fix: remove test button assertion

* fix: fix type imports

* test: attempt fixing unit tests

* fix: changed to using useToast for error toasts

* Minor copy tweaks and swapped buttons position.

* fix locale ref

* align error with UI wording

* simplify saving ux

* fix pretty

* fix: update saml sso setting saving

* fix: undo try/catch changes when saving saml config

* metadata url tab selected at first

* chore: fix linting issue

* test: fix activation checkbox test

---------

Co-authored-by: Giulio Andreini <g.andreini@gmail.com>
Co-authored-by: Michael Auerswald <michael.auerswald@gmail.com>
Co-authored-by: Romain Minaud <romain.minaud@gmail.com>

* fix(editor): Fix canvas loading when page gets restored from bfcache (n8n-io#6304)

* fix(editor): Fix canvas loading when page gets restored from bfcache

* Lint fix

* fix(editor): Link to log streaming doc from log streaming (no-changelog) (n8n-io#6303)

link to log streaming doc from log streaming

---------

Co-authored-by: Eyal Fishler <eyalfishler@gmail.com>
Co-authored-by: Jon <jonathan.bennetts@gmail.com>
Co-authored-by: Deborah <deborah@starfallprojects.co.uk>
Co-authored-by: agobrech <45268029+agobrech@users.noreply.github.com>
Co-authored-by: Marcus <marcus@n8n.io>
Co-authored-by: Alex Grozav <alex@grozav.com>
Co-authored-by: Omri Attoun <omri@joyous.team>
Co-authored-by: Michael Auerswald <michael.auerswald@gmail.com>
Co-authored-by: Michael Kret <88898367+michael-radency@users.noreply.github.com>
Co-authored-by: Omar Ajoue <krynble@gmail.com>
Co-authored-by: Yann ALEMAN <yann.aleman27@gmail.com>
Co-authored-by: Giulio Andreini <g.andreini@gmail.com>
Co-authored-by: Romain Minaud <romain.minaud@gmail.com>
Co-authored-by: OlegIvaniv <me@olegivaniv.com>
@janober
Copy link
Member

janober commented May 25, 2023

Got released with n8n@0.230.1

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 Released ui Enhancement in /editor-ui or /design-system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants