Description
Tomás Cohen Arazi (tcohen)
2024-07-29 15:08:10 UTC
Created attachment 170475 [details] [review] Bug 37513: Handle Koha::Exceptions::Object::FKConstraintDeletio in controller Created attachment 170476 [details] [review] Bug 37513: Add Koha::RecordSource->usage_count This patch adds the `usage_count` method to be used for embedding from the API. Tests cover all use cases. To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/RecordSource.t => SUCCESS: Tests pass! 3. Sign off :-D Created attachment 170477 [details] [review] Bug 37513: Allow embedding usage_count in record sources Created attachment 170478 [details] [review] Bug 37513: Add a way for KohaTable to decide if a button should be displayed This patch adds, for the case of object parameters in the actions config entry for the KohaTable Vue component, a check for a function on the current row, that will be used to determine if a button needs to be displayed or not. It checks for the function to be defined, and uses it for checking displayability. Created attachment 170479 [details] [review] Bug 37513: Only display delete button if usage_count == 0 Created attachment 170512 [details] [review] Bug 37513: Handle Koha::Exceptions::Object::FKConstraintDeletio in controller Signed-off-by: David Nind <david@davidnind.com> Created attachment 170513 [details] [review] Bug 37513: Add Koha::RecordSource->usage_count This patch adds the `usage_count` method to be used for embedding from the API. Tests cover all use cases. To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/RecordSource.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Created attachment 170514 [details] [review] Bug 37513: Allow embedding usage_count in record sources Signed-off-by: David Nind <david@davidnind.com> Created attachment 170515 [details] [review] Bug 37513: Add a way for KohaTable to decide if a button should be displayed This patch adds, for the case of object parameters in the actions config entry for the KohaTable Vue component, a check for a function on the current row, that will be used to determine if a button needs to be displayed or not. It checks for the function to be defined, and uses it for checking displayability. Signed-off-by: David Nind <david@davidnind.com> Created attachment 170516 [details] [review] Bug 37513: Only display delete button if usage_count == 0 Signed-off-by: David Nind <david@davidnind.com> Testing notes (using KTD): 1. Add a record source using the staff interface: 1.1 Go to Administration > Catalog > Record sources > + New record source 1.2 Name the new record source TEST 2. Update a record so that it uses this new record source: 2.1 The record source ID is listed in the record sources table ID column 2.2 Update record 262 in the database: - koha-mysql kohadev - UPDATE biblio_metadata SET record_source_id=1 WHERE biblionumber=262; 3. Go to delete the record source - note that you CANNOT delete the record source and get an error message, as there are records associated with it: 3.1 From the record sources table, select the 'Delete' action for the TEST record source 3.2 You are prompted: Are you sure you want to delete this record source? TEST Yes, delete No, do not delete 3.3 Select 'Yes, delete' 3.4 You get an error message: "Something went wrong: Error: Something went wrong, check Koha logs for details." 4. Apply the patches. 5. Update the view components: yarn install 6. Restart everything: restart_all 7. Check that the new tests pass: prove t/db_dependent/Koha/RecordSource.t 8. Refresh the record sources page - note that there is now no 'Delete' action. 9. Update the bibliographic record you added the record source to so that it no longer has a record source: 9.1 koha-mysql kohadev 9.2 UPDATE biblio_metadata SET record_source_id=NULL WHERE biblionumber=262; 10. Refresh the record sources page 11. Note that: - there is now a 'Delete' action - you can now delete the record source - the result message is: Record source 'TEST' deleted Created attachment 170544 [details] [review] Bug 37513: Handle Koha::Exceptions::Object::FKConstraintDeletio in controller Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> Created attachment 170545 [details] [review] Bug 37513: Add Koha::RecordSource->usage_count This patch adds the `usage_count` method to be used for embedding from the API. Tests cover all use cases. To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/RecordSource.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> Created attachment 170546 [details] [review] Bug 37513: Allow embedding usage_count in record sources Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> Created attachment 170547 [details] [review] Bug 37513: Add a way for KohaTable to decide if a button should be displayed This patch adds, for the case of object parameters in the actions config entry for the KohaTable Vue component, a check for a function on the current row, that will be used to determine if a button needs to be displayed or not. It checks for the function to be defined, and uses it for checking displayability. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> Created attachment 170548 [details] [review] Bug 37513: Only display delete button if usage_count == 0 Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> If I see this correctly, there is no hint to the user why the option is missing. Maybe we could add a usage counter to the page as a little hint as we look it up? Used in: ... record(s) Pushed for 24.11! Well done everyone, thank you! Cypress tests are now failing. 3) Record sources CRUD tests Delete: AssertionError: Timed out retrying after 10000ms: Expected to find content: 'Delete' within the element: <tr.even> but never did. Created attachment 171205 [details] [review] Bug 37513: Fix and add more Cypress tests Hm, the follow-up still fails for me :( 1) Record sources CRUD tests Delete: TypeError: The following error originated from your application code, not from Cypress. It was caused by an unhandled promise rejection. > $(...).tooltip is not a function When Cypress detects uncaught errors originating from your application it will automatically fail the current test. This behavior is configurable, and you can choose to turn this off by listening to the `uncaught:exception` event. https://on.cypress.io/uncaught-exception-from-application at Proxy.beforeMount (webpack://koha/./koha-tmpl/intranet-tmpl/prog/js/vue/components/KohaTable.vue?../node_modules/vue-loader/dist/index.js??ruleSet%5B1%5D.rules%5B5%5D.use%5B0%5D:142:11) at eval (webpack://koha/../node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js?:2849:40) at callWithErrorHandling (webpack://koha/../node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js?:284:22) at callWithAsyncErrorHandling (webpack://koha/../node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js?:293:21) at hook.__weh.hook.__weh (webpack://koha/../node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js?:2823:29) at invokeArrayFns (webpack://koha/../node_modules/@vue/shared/dist/shared.esm-bundler.js?:625:15) at ReactiveEffect.componentUpdateFn [as fn] (webpack://koha/../node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js?:5688:80) at ReactiveEffect.run (webpack://koha/../node_modules/@vue/reactivity/dist/reactivity.esm-bundler.js?:225:25) at instance.update (webpack://koha/../node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js?:5844:56) at setupRenderEffect (webpack://koha/../node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js?:5858:9) Interestingly the error disappeared on the second run and I can't make it re-appear. Pushed the follow-up "Fix and add more Cypress tests" to main. I got the same errors for RecordSources_spec.ts as in comment #21). When running all the Cypress tests, I got this straight after it starts - not sure whether this is useful or not: ------------------ perl /kohadevbox/misc4dev/run_tests.pl --run-cypress-tests-only Running [koha-shell kohadev -c "SELENIUM_PORT=4444 KOHA_INTRANET_URL=http://koha:8081 SELENIUM_ADDR=selenium KOHA_USER=koha KOHA_TESTING=1 NODE_PATH=/kohadevbox/node_modules JUNIT_OUTPUT_FILE=junit_main.xml KOHA_OPAC_URL=http://koha:8080 KOHA_PASS=koha KOHA_NO_TABLE_LOCKS=1 PATH=/usr/bin:/bin:/usr/sbin:/sbin:/kohadevbox/bin:/kohadevbox/koha/node_modules/.bin/:/kohadevbox/node_modules/.bin/ yarn cypress run --config video=false,screenshotOnRunFailure=false --env KOHA_USER=koha,KOHA_PASS=koha --reporter junit --reporter-options 'mochaFile=junit-cypress-[hash].xml,toConsole=true'";err=$?; if [ $err -eq 0 ]; then echo all good; elif [ $err -eq 127 ]; then echo '<?xml version="1.0" encoding="UTF-8"?><testsuites name="Cypress run" time="0.0000" tests="1" failures="1"><testsuite name="Root Suite" timestamp="2024-09-09T20:52:28+00:00" tests="0" file="" time="0.0000" failures="1"><testcase name="Executable not found"><failure message="Cypress executable not found." type="AssertionError">Cypress executable not found!</failure></testcase></testsuite></testsuites>' > junit-cypress-exec.xml; else echo "Cypress returned error code '$err'"; fi; exit $err; ]... yarn run v1.22.22 $ cypress run --config video=false,screenshotOnRunFailure=false --env KOHA_USER=koha,KOHA_PASS=koha --reporter junit --reporter-options mochaFile=junit-cypress-[hash].xml,toConsole=true DevTools listening on ws://127.0.0.1:42441/devtools/browser/3585392f-09c7-4db1-8145-881da9f2eb62 [4212:0909/205229.359611:ERROR:gpu_memory_buffer_support_x11.cc(44)] dri3 extension not supported. Missing baseUrl in compilerOptions. tsconfig-paths will be skipped Couldn't determine Mocha version .... Messages the same as on comment #21. ..... Spec Tests Passing Failing Pending Skipped ✖ Admin/RecordSources_spec.ts 00:08 5 4 1 - - ✖ 1 of 17 failed (6%) 04:43 85 84 1 - - error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. Cypress returned error code '1' ------------------ Things I did: 1. I removed all the docker images/volumes and did a fresh pull. 2. Started up KTD with Selenium: ktd --selenium up 3. Accessed the Koha shell as root: ktd --root --shell 4. Ran all the Cypress tests: 4.1 cd koha 4.2 perl /kohadevbox/misc4dev/run_tests.pl --run-cypress-tests-only 5. The tests failed as above. 6. I repeated step 4.2.: 6.1 The first three times, they failed. 6.2 The fourth time, they passed. 6.3 The fifth time, they failed 7. Running just the single test (ktd --shell (not as root)) - yarn cypress run --spec t/cypress/integration/Admin/RecordSources_spec.ts: 7.1 First time it passes 7.2 Second time it fails 7.3 Third to eighth time it passes 7.4 I didn't do any more... I'll attach the output from the failure. Not sure if this helps! Created attachment 171236 [details]
Output from test failure for RecordSources (first tme success - second time failed)
It started to work for me with the second run and all runs after - this doesn't work for you? Hm. OK - I see it didn't. Back to Tomas. (In reply to Katrin Fischer from comment #26) > Hm. OK - I see it didn't. Back to Tomas. Resolved since. Changing status to needs documenting, as this includes a user interface change and behavour change that needs including in the manual (https://koha-community.org/manual/latest/en/html/administration.html#record-sources) |