When dropping an unsupported file onto a notebook entry, tell the user it isnt supported #7115

scottbell · 2023-10-05T19:02:47Z

Describe your changes:

I've added notifications & errors for:

Unknown files
Couch errors on initial save of image (e.g., too big)
Annotation causes image to enlarge and fails to update

I've also updated the README in the CouchDB plugin to describe how to up the documents limits. Basically:

  [couchdb]
  max_document_size = 4294967296 ; approx 4 GB
  
  [chttpd]
  max_http_request_size = 4294967296 ; approx 4 GB

as both the documents, and the HTTP server in front of CouchDB need to accept the new big limits.

I've also updated interaction with the entry to accept pasting of images, both in the text box, and if selecting the entry.

All Submissions:

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?
Is this change backwards compatible? For example, developers won't need to change how they are calling the API or how they've extended core plugins such as Tables or Plots.

Author Checklist

Changes address original issue?
Tests included and/or updated with changes?
Command line build passes?
Has this been smoke tested?
Testing instructions included in associated issue OR is this a dependency/testcase change?

Reviewer Checklist

Changes appear to address issue?
Reviewer has tested changes by following the provided instructions?
Changes appear not to be breaking changes?
Appropriate automated tests included?
Code style and in-line documentation are appropriate?
Has associated issue been labelled unverified? (only applicable if this PR closes the issue)
Has associated issue been labelled bug? (only applicable if this PR is for a bug fix)

deploysentinel · 2023-10-05T19:07:33Z

Current Playwright Test Results Summary

✅ 14 Passing

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 10/11/2023 07:17:59am UTC)

Run Details

Running Workflow e2e-couchdb on Github Actions

Commit: 391ea20

Started: 10/11/2023 07:16:04am UTC

View Detailed Build Results

Current Playwright Test Results Summary

✅ 143 Passing - ⚠️ 3 Flaky

Run may still be in progress, this comment will be updated as current testing workflow or job completes...

(Last updated on 10/11/2023 07:17:59am UTC)

Run Details

Running Job e2e-stable on CircleCI

Commit: 391ea20

Started: 10/11/2023 07:05:11am UTC

⚠️ Flakes

📄 functional/plugins/notebook/restrictedNotebook.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Restricted Notebook with a page locked and with an embed @addinit Disallows embeds to be deleted if page locked @addinit Retry 1 • Initial Attempt	0% (0) 0 / 57 runs failed over last 7 days	33.33% (19) 19 / 57 runs flaked over last 7 days

📄 functional/planning/timelist.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Time List Create a Time List, add a single Plan to it and verify all the activities are displayed with no milliseconds Retry 1 • Initial Attempt	2.06% (2) 2 / 97 runs failed over last 7 days	62.89% (61) 61 / 97 runs flaked over last 7 days

📄 functional/plugins/notebook/notebookSnapshots.e2e.spec.js • 1 Flake

Test Case Results

Test Case	Last 7 days Failures	Last 7 days Flakes
Snapshot image tests Can drop an image onto a notebook and create a new entry Retry 1 • Initial Attempt	3.08% (2) 2 / 65 runs failed over last 7 days	58.46% (38) 38 / 65 runs flaked over last 7 days

View Detailed Build Results

codecov · 2023-10-06T09:59:54Z

Codecov Report

Merging #7115 (391ea20) into master (a16a1d3) will decrease coverage by 0.36%.
The diff coverage is 21.87%.

@@            Coverage Diff             @@
##           master    #7115      +/-   ##
==========================================
- Coverage   55.77%   55.42%   -0.36%     
==========================================
  Files         651      651              
  Lines       26114    26170      +56     
  Branches     2521     2535      +14     
==========================================
- Hits        14566    14504      -62     
- Misses      10847    10973     +126     
+ Partials      701      693       -8

Flag	Coverage Δ		*Carryforward flag
e2e-full	`41.92% <ø> (+0.03%)`	⬆️	Carriedforward from a16a1d3
e2e-stable	`57.26% <54.16%> (-0.02%)`	⬇️
unit	`48.84% <3.12%> (-0.50%)`	⬇️

*This pull request uses carry forward flags. Click here to find out more.

Files	Coverage Δ
src/plugins/notebook/utils/notebook-entries.js	`84.67% <100.00%> (+0.37%)`	⬆️
src/plugins/plot/MctPlot.vue	`36.26% <0.00%> (-0.10%)`	⬇️
src/plugins/notebook/utils/notebook-image.js	`40.00% <0.00%> (-11.86%)`	⬇️
.../plugins/notebook/components/NotebookComponent.vue	`17.89% <5.88%> (-0.54%)`	⬇️
src/plugins/notebook/components/NotebookEntry.vue	`15.42% <0.00%> (-3.17%)`	⬇️

... and 9 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a16a1d3...391ea20. Read the comment docs.

ozyx

Looking good so far!

I found a few issues while testing:

When creating an entry with an image embed (dragging the image onto the "create new entry" area), the "create new entry" button is disabled until we add text to the entry. I think this behavior is intended (we don't want entries w/ only embeds and no text), but we should probably focus the entry input after drag+drop is done. Also, we are still able to have entries with embeds and no text after clicking and confirming the entry. @charlesh88 that's a no-no, right?
Dragging an image onto an existing entry with no embeds fails here due to this.entry.embeds === null.

Uncaught (in promise) TypeError: Cannot read properties of null (reading 'push')
    at eval (NotebookEntry.vue:488:33)
    at async Promise.all (:8080/index 0)
    at async Proxy.dropOnEntry (NotebookEntry.vue:479:9)

For the life of me I cannot get image copy/paste into an entry to work. Nothing happens? I might be doing something wrong.

This recording covers cases 1 and 2:

Screen.Recording.2023-10-06.at.11.20.43.AM.mov

src/plugins/persistence/couch/README.md

src/plugins/notebook/utils/notebook-entries.js

e2e/tests/functional/plugins/notebook/notebook.e2e.spec.js

…o-a-notebook-entry-tell-the-user-it-isnt-supported

scottbell · 2023-10-10T10:21:17Z

@ozyx I think I've addressed 1 (though not sure about empty text), and 2. For 3, you need to select an entry for paste to work, either the entry as a whole, or in the text. Here's a video of the fixes:

Screen.Recording.2023-10-10.at.12.09.44.PM.mov

…an-unsupported-file-onto-a-notebook-entry-tell-the-user-it-isnt-supported

ozyx

Awesome work here. Tested this (and stress tested with localStorage / CouchDB plugins) and this works well! LGTM

ozyx · 2023-10-12T04:42:57Z

src/plugins/notebook/components/NotebookComponent.vue

-      const localImageDropped = dropEvent.dataTransfer.files?.[0]?.type.includes('image');
-      const imageUrl = dropEvent.dataTransfer.getData('URL');
+      const dataTransferFiles = Array.from(dropEvent.dataTransfer.files);
+      const localImageDropped = dataTransferFiles.some((file) => file.type.includes('image'));


awesome refactor

ozyx · 2023-10-12T04:45:35Z

src/plugins/notebook/components/NotebookComponent.vue

+          dataTransferFiles.map(async (file) => {
+            if (file.type.includes('image')) {
+              const imageData = file;
+              const imageEmbed = await createNewImageEmbed(


Good candidate for a refactor with composables! foreshadowing intensifies

scottbell added 4 commits October 5, 2023 13:19

handle unknown files and deal with copy/paste

c74b6c5

add some nice errors if couch pukes

f90ae48

added how to adjust couchdb limits

07e88ba

throw error on anntotation change

3717e7a

scottbell linked an issue Oct 5, 2023 that may be closed by this pull request

When dropping an unsupported file onto a notebook entry, tell the user it isn't supported #7097

Closed

scottbell changed the title ~~7097 when dropping an unsupported file onto a notebook entry tell the user it isnt supported~~ When dropping an unsupported file onto a notebook entry, tell the user it isnt supported Oct 5, 2023

scottbell mentioned this pull request Oct 6, 2023

[Notebook] Fixes for Markdown in Notebook entries #7119

Closed

7 tasks

keep blockquotes

9d79092

scottbell added 3 commits October 6, 2023 12:10

add test for blockquotes

3114a5c

allow multi-file drop of images too

8ef6f94

add test for big files

6d34c51

scottbell linked an issue Oct 6, 2023 that may be closed by this pull request

[Notebook] Fixes for Markdown in Notebook entries #7119

Closed

7 tasks

scottbell added 4 commits October 6, 2023 16:10

spell check

d544361

check for null

eeb92ac

need to ignore console errors

9be2086

reorder tests so we can ignore console errors

f8cf49b

scottbell added the pr:e2e:couchdb npm run test:e2e:couchdb label Oct 6, 2023

github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Oct 6, 2023

scottbell marked this pull request as ready for review October 6, 2023 16:08

scottbell requested review from ozyx and jvigliotta October 6, 2023 16:08

ozyx suggested changes Oct 6, 2023

View reviewed changes

src/plugins/persistence/couch/README.md Show resolved Hide resolved

src/plugins/notebook/utils/notebook-entries.js Show resolved Hide resolved

e2e/tests/functional/plugins/notebook/notebook.e2e.spec.js Show resolved Hide resolved

scottbell added 3 commits October 10, 2023 11:00

Merge branch 'master' into 7097-when-dropping-an-unsupported-file-ont…

5718bc8

…o-a-notebook-entry-tell-the-user-it-isnt-supported

when creating new entry, ready it for editing

5c24869

fix tests and empty embeds

9510d92

fix tests

38ff960

scottbell added the pr:e2e:couchdb npm run test:e2e:couchdb label Oct 10, 2023

scottbell requested a review from ozyx October 10, 2023 10:45

github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Oct 10, 2023

found similar issue from notebooks in plots

d383232

scottbell added the pr:e2e:couchdb npm run test:e2e:couchdb label Oct 10, 2023

github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Oct 10, 2023

Merge remote-tracking branch 'origin/master' into 7097-when-dropping-…

bb3d1a8

…an-unsupported-file-onto-a-notebook-entry-tell-the-user-it-isnt-supported

scottbell added the pr:e2e:couchdb npm run test:e2e:couchdb label Oct 10, 2023

github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Oct 10, 2023

Merge remote-tracking branch 'origin/master' into 7097-when-dropping-…

391ea20

…an-unsupported-file-onto-a-notebook-entry-tell-the-user-it-isnt-supported

scottbell added the pr:e2e:couchdb npm run test:e2e:couchdb label Oct 11, 2023

github-actions bot removed the pr:e2e:couchdb npm run test:e2e:couchdb label Oct 11, 2023

ozyx approved these changes Oct 12, 2023

View reviewed changes

scottbell merged commit 26d3bd1 into master Oct 12, 2023
11 checks passed

scottbell deleted the 7097-when-dropping-an-unsupported-file-onto-a-notebook-entry-tell-the-user-it-isnt-supported branch October 12, 2023 05:34

unlikelyzero added this to the Target:3.2.0 milestone Nov 13, 2023

davetsay mentioned this pull request Apr 12, 2024

Paste frequently does not work in notebook entries #7686

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

When dropping an unsupported file onto a notebook entry, tell the user it isnt supported #7115

When dropping an unsupported file onto a notebook entry, tell the user it isnt supported #7115

scottbell commented Oct 5, 2023 •

edited

Loading

deploysentinel bot commented Oct 5, 2023 •

edited

Loading

Test Case Results

Test Case Results

Test Case Results

codecov bot commented Oct 6, 2023 •

edited

Loading

ozyx left a comment •

edited

Loading

scottbell commented Oct 10, 2023

ozyx left a comment

ozyx Oct 12, 2023

ozyx Oct 12, 2023

When dropping an unsupported file onto a notebook entry, tell the user it isnt supported #7115

When dropping an unsupported file onto a notebook entry, tell the user it isnt supported #7115

Conversation

scottbell commented Oct 5, 2023 • edited Loading

Describe your changes:

All Submissions:

Author Checklist

Reviewer Checklist

deploysentinel bot commented Oct 5, 2023 • edited Loading

Current Playwright Test Results Summary

Current Playwright Test Results Summary

⚠️ Flakes

Test Case Results

Test Case Results

Test Case Results

codecov bot commented Oct 6, 2023 • edited Loading

Codecov Report

ozyx left a comment • edited Loading

Choose a reason for hiding this comment

scottbell commented Oct 10, 2023

ozyx left a comment

Choose a reason for hiding this comment

ozyx Oct 12, 2023

Choose a reason for hiding this comment

ozyx Oct 12, 2023

Choose a reason for hiding this comment

scottbell commented Oct 5, 2023 •

edited

Loading

deploysentinel bot commented Oct 5, 2023 •

edited

Loading

codecov bot commented Oct 6, 2023 •

edited

Loading

ozyx left a comment •

edited

Loading