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

[stable25] Fix direct editing loading of txt files #3479

Merged
merged 2 commits into from
Nov 24, 2022

Conversation

juliusknorr
Copy link
Member

Backport of #3476

@juliusknorr juliusknorr added bug Something isn't working 3. to review labels Nov 24, 2022
@cypress
Copy link

cypress bot commented Nov 24, 2022



Test summary

98 0 0 0Flakiness 1


Run details

Project Text
Status Passed
Commit 93e7f6d ℹ️
Started Nov 24, 2022 3:04 PM
Ended Nov 24, 2022 3:10 PM
Duration 06:18 💡
OS Linux Ubuntu - 20.04
Browser Electron 106

View run in Cypress Dashboard ➡️


Flakiness

cypress/e2e/links.spec.js Flakiness
1 test link marks > link preview > shows a link preview

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

this.emit('loaded', {
document: this.document,
session: this.session,
documentSource: '' + content,
documentSource: '' + connectionData.content,
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this the same thing we removed above? I'm confused. 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, which is fine still just without the fetch fallback

Copy link
Member Author

Choose a reason for hiding this comment

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

The main problem was that an empty string (from a file without content) would always trigger the fetch request which is actually not necessary anymore as we always pass the content earlier.

this.emit('loaded', {
document: this.document,
session: this.session,
documentSource: '' + content,
documentSource: '' + connectionData.content,
Copy link
Contributor

@Raudius Raudius Nov 24, 2022

Choose a reason for hiding this comment

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

Minor nitpick, but could we just cast it?

documentSource: String(connectionData.content)

Copy link
Contributor

Choose a reason for hiding this comment

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

Nevermind, this is already a backport 🙈

Approved

Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr
Copy link
Member Author

/compile

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@juliusknorr juliusknorr merged commit dd3f541 into stable25 Nov 24, 2022
@juliusknorr juliusknorr deleted the backport/3476/stable25 branch November 24, 2022 15:14
@blizzz blizzz mentioned this pull request Nov 25, 2022
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants