samvera / browse-everything

Rails engine providing access to files in cloud storage
Apache License 2.0
114 stars 22 forks source link

Tweak some javascript problems #404

Closed jrochkind closed 2 years ago

jrochkind commented 2 years ago

While CI isn't currently running JS feature tests (working on it in #400), I am able to run them locally (and have them be green -- both before and after this PR). These fixes came from such, as well as from manually running the engine_cart .internal_test_app and manipulating it in manual tests.

"(No Turbolinks)" feature test path wasn't, and wasn't any different -- plus failed when fixed

Feature tests tried to test a "No Turbolinks" test path, by clicking on a link with "data-no-turbolink" attribute

Two problems with this:

  1. That attribute has no effect on recent versions of Turbolinks that were being used in tests, it needs to be data-turbolinks="false" instead. So this test path was not doing anything different at all from the "(Turbolinks)" test path.
  2. Technically even once working, it's nto testing "no turbolinks", it's testing: Turbolinks is loaded, and page with b-e is the FIRST page in the turbolinks session, so loaded with a full load.

I fixed (1). While it's not "no turbolinks" it turns out that IS a test path we want to test, because once it was testing the "first load with turbolinks" condition, a previously green test went red!

I fixed that by ensuring some JS code that was waiting only on turbolinks:load would wait BOTH on turbolinks:load and ready. In "first page load" scenario, turbolinks:load could happen before ready, and the triggered code was firing before structures it needed were set up.

CSRF failure in test app setup

The current test app setup does some frankenstein things to set up a test app that can deliver b-e JS with webpack: It copies some JS source into local app, and then even modifies those files a bit to add some require/imports to refer to dependencies for webpack.

When i ran the .internal_test_app manually, it was failing to include a proper CSRF token when POST'ing back to app, and thus failing the CSRF check and not fully succeeding in browse-everything file selection.

The reason was because it was trying to find a CSRF token to include with request using a Rails JS object, only if the Rails JS object was present -- but in webpack environment, to be able to refer to Rails you have to import it.

So I added that import to the test_app_generator. Now, manually running the engine-cart test app, and manually testing it -- b-e file selection works to completion, with no Rails HTTP status 422 CSRF failure.

jrochkind commented 2 years ago

While CI isn't running the feature test, I have confirmed: