Implement delivering client assets for custom elements in shared questions #9786

SethPoulsen · 2024-04-29T21:40:04Z

partial replacement for #9776

resolves part of #9397

github-actions · 2024-04-29T21:50:09Z

All images

Image	Platform	Old Size	New Size	Change
prairielearn/executor:4c4ae2d8900cc97c9dd39a5d6e12b191f92f8813	null	1187.93 MB	1187.78 MB	-0.01%
prairielearn/prairielearn:4c4ae2d8900cc97c9dd39a5d6e12b191f92f8813	linux/amd64	1187.92 MB	1187.78 MB	-0.01%

SethPoulsen · 2024-04-29T22:39:03Z

Do we have any automated tests for the elementFiles endpoint that I should be updating for this?

jonatanschroeder

I don't think there is an easy solution to the questions I'm bringing here, we can discuss it in our meeting.

apps/prairielearn/src/question-servers/freeform.js

apps/prairielearn/src/pages/elementFiles/elementFiles.js

apps/prairielearn/src/question-servers/freeform.js

docs/questionSharing.md

Co-authored-by: Jonatan Schroeder <jonatan@yorku.ca>

SethPoulsen · 2024-05-02T18:02:57Z

I'm getting a circular dependency error now:

error no-circular: apps/prairielearn/src/lib/assets.ts → 
      apps/prairielearn/src/pages/elementFiles/elementFiles.js →
      apps/prairielearn/src/lib/question-variant.ts →
      apps/prairielearn/src/question-servers/index.ts →
      apps/prairielearn/src/question-servers/freeform.js →
      apps/prairielearn/src/lib/assets.ts

This was brought about by the fact that I am now using getQuestionCourse in elementFiles.js, which has to be imported from question-variant.ts.

Anyone have recommendations on fixing?

I was surprised that assets.ts is importing elementFiles.js this appears to be so that it can add or remove asset cacheing based on being in dev mode or production mode.

apps/prairielearn/src/question-servers/freeform.js

jonatanschroeder · 2024-05-03T19:58:32Z

Anyone have recommendations on fixing?

What I've done in the past to fix these was to split a library in two.

I was surprised that assets.ts is importing elementFiles.js this appears to be so that it can add or remove asset cacheing based on being in dev mode or production mode.

I suggest moving the applyMiddleware function in assets.ts to a separate file, possibly in middlewares.

apps/prairielearn/src/pages/elementFiles/elementFiles.sql

nwalters512

Make sure you reformat these files to reorder imports once you resolve the merge conflicts!

apps/prairielearn/src/server.js

apps/prairielearn/src/tests/accessibility/index.test.ts

apps/prairielearn/src/tests/helperQuestionPreview.ts

apps/prairielearn/src/tests/sharedQuestionPreview.test.ts

apps/prairielearn/src/pages/elementFiles/elementFiles.sql

codecov · 2024-05-27T15:47:36Z

Codecov Report

Attention: Patch coverage is 89.88764% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 67.33%. Comparing base (d6382d6) to head (4c4ae2d).

Files	Patch %	Lines
apps/prairielearn/src/server.js	75.00%	0 Missing and 6 partials ⚠️
...rairielearn/src/pages/elementFiles/elementFiles.js	86.95%	3 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #9786   +/-   ##
=======================================
  Coverage   67.32%   67.33%           
=======================================
  Files         458      458           
  Lines       71419    71503   +84     
  Branches     5756     5770   +14     
=======================================
+ Hits        48081    48144   +63     
- Misses      22909    22921   +12     
- Partials      429      438    +9

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

SethPoulsen · 2024-05-27T16:43:21Z

apps/prairielearn/src/tests/sharedQuestionPreview.test.ts

+  describe('Shared Question Previews Within a Course Instance', function () {
+    const previewPageInfo = {
+      siteUrl,
+      baseUrl,
+      questionBaseUrl: baseUrl + `/course_instance/2/instructor/question`,
+      questionPreviewTabUrl: '/preview',
+      isStudentPage: false,
+    };
+
+    testQuestionPreviews(previewPageInfo, addNumbers, addVectors);
+
+    testFileDownloads(previewPageInfo, downloadFile, false);
+
+    testElementClientFiles(previewPageInfo, customElement);
+  });
+});


In addition to resolving all the PR comments, the latest commits also added these few lines to test question previews under course_instance urls, another thing we haven't had any tests for before now

nwalters512

No blockers, but a number of nits that would be nice to address.

apps/prairielearn/src/pages/elementFiles/elementFiles.js

apps/prairielearn/src/question-servers/freeform.js

apps/prairielearn/src/tests/helperQuestionPreview.ts

apps/prairielearn/src/tests/instructorQuestions.test.ts

apps/prairielearn/src/server.js

apps/prairielearn/src/pages/elementFiles/elementFiles.sql

SethPoulsen added 2 commits April 26, 2024 13:49

share a question to example course with client element code

13138c2

get it working

2ae72ed

SethPoulsen changed the title ~~Implment delivering client assets for custom elements in shared questions~~ Implement delivering client assets for custom elements in shared questions Apr 29, 2024

Merge branch 'master' into sharing-element-client-code

b0cf027

SethPoulsen mentioned this pull request Apr 29, 2024

Question Sharing: Path out of Beta #9397

Open

16 tasks

SethPoulsen added 3 commits April 29, 2024 16:28

fix accessibility tests, get rid of unneeded comments

d22a818

restore original links

2976251

Remove question added to infoAssessment.json for testing

4ff3e98

SethPoulsen marked this pull request as ready for review April 29, 2024 22:38

SethPoulsen requested review from jonatanschroeder and mwest1066 April 29, 2024 22:38

jonatanschroeder reviewed Apr 30, 2024

View reviewed changes

apps/prairielearn/src/question-servers/freeform.js Outdated Show resolved Hide resolved

apps/prairielearn/src/pages/elementFiles/elementFiles.js Outdated Show resolved Hide resolved

SethPoulsen added 5 commits April 30, 2024 14:39

Merge branch 'master' into sharing-element-client-code

853d06b

address PR comments

67a122f

update sharing docs

b6352e9

comment

d9bc072

revert unrelated change

f408f7f

jonatanschroeder reviewed Apr 30, 2024

View reviewed changes

apps/prairielearn/src/question-servers/freeform.js Outdated Show resolved Hide resolved

jonatanschroeder reviewed Apr 30, 2024

View reviewed changes

docs/questionSharing.md Outdated Show resolved Hide resolved

SethPoulsen and others added 2 commits May 2, 2024 11:11

Update docs/questionSharing.md

ced926e

Co-authored-by: Jonatan Schroeder <jonatan@yorku.ca>

fixes based on meeting discussion

27be23e

SethPoulsen requested a review from jonatanschroeder May 2, 2024 17:14

lint fix

5a621df

mwest1066 reviewed May 3, 2024

View reviewed changes

apps/prairielearn/src/question-servers/freeform.js Outdated Show resolved Hide resolved

SethPoulsen added 2 commits May 7, 2024 09:31

Merge branch 'master' into sharing-element-client-code

b8361d4

refactor applyMiddleware out of lib/assets

e62e1c5

SethPoulsen added 2 commits May 15, 2024 08:01

change url structure

4046f30

get rid of asset refactor now that circular dependency is gone

6589c49

SethPoulsen commented May 15, 2024

View reviewed changes

apps/prairielearn/src/pages/elementFiles/elementFiles.sql Show resolved Hide resolved

SethPoulsen requested a review from nwalters512 May 15, 2024 14:11

SethPoulsen added 3 commits May 15, 2024 09:11

Merge branch 'master' into sharing-element-client-code

9463525

remove unneeded file

7ccb74b

restore accidentally deleted line

a6cdf95

nwalters512 reviewed May 16, 2024

View reviewed changes

Merge branch 'master' into sharing-element-client-code

9a42132

SethPoulsen added 2 commits May 27, 2024 10:11

respond to pr comments, add more tests

bb4ce25

pr comments

3dbad7f

SethPoulsen commented May 27, 2024

View reviewed changes

SethPoulsen requested a review from nwalters512 May 27, 2024 16:44

get rid of unwanted log

19c4c84

SethPoulsen mentioned this pull request May 27, 2024

Implement delivering client assets for custom elements in public question previews #9807

Merged

SethPoulsen added 3 commits May 28, 2024 10:20

fix url

8ad37bb

fix urls in accessibility tests

40c9983

Merge branch 'master' into sharing-element-client-code

e8ce499

nwalters512 approved these changes May 28, 2024

View reviewed changes

SethPoulsen added 2 commits May 28, 2024 16:12

Merge branch 'master' into sharing-element-client-code

716bd13

address PR comments

cb4b4c4

nwalters512 reviewed May 28, 2024

View reviewed changes

apps/prairielearn/src/pages/elementFiles/elementFiles.sql Show resolved Hide resolved

SethPoulsen added 2 commits May 28, 2024 16:35

check for question deletion

45a4b86

Merge branch 'master' into sharing-element-client-code

4c4ae2d

SethPoulsen enabled auto-merge May 28, 2024 22:40

SethPoulsen added this pull request to the merge queue May 28, 2024

Merged via the queue into master with commit c15fce5 May 28, 2024
9 checks passed

SethPoulsen deleted the sharing-element-client-code branch May 28, 2024 22:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement delivering client assets for custom elements in shared questions #9786

Implement delivering client assets for custom elements in shared questions #9786

SethPoulsen commented Apr 29, 2024 •

edited

github-actions bot commented Apr 29, 2024 •

edited

SethPoulsen commented Apr 29, 2024

jonatanschroeder left a comment

SethPoulsen commented May 2, 2024

jonatanschroeder commented May 3, 2024

nwalters512 left a comment

codecov bot commented May 27, 2024 •

edited

SethPoulsen May 27, 2024

nwalters512 left a comment

Implement delivering client assets for custom elements in shared questions #9786

Implement delivering client assets for custom elements in shared questions #9786

Conversation

SethPoulsen commented Apr 29, 2024 • edited

github-actions bot commented Apr 29, 2024 • edited

All images

SethPoulsen commented Apr 29, 2024

jonatanschroeder left a comment

Choose a reason for hiding this comment

SethPoulsen commented May 2, 2024

jonatanschroeder commented May 3, 2024

nwalters512 left a comment

Choose a reason for hiding this comment

codecov bot commented May 27, 2024 • edited

Codecov Report

SethPoulsen May 27, 2024

Choose a reason for hiding this comment

nwalters512 left a comment

Choose a reason for hiding this comment

SethPoulsen commented Apr 29, 2024 •

edited

github-actions bot commented Apr 29, 2024 •

edited

codecov bot commented May 27, 2024 •

edited