37513 – Disable 'Delete' button if the record source cannot be deleted

Bug 37513 - Disable 'Delete' button if the record source cannot be deleted

Summary: Disable 'Delete' button if the record source cannot be deleted

Status:	Pushed to main

Alias:	None

Product:	Koha
Classification:	Unclassified
Component:	System Administration (show other bugs)
Version:	unspecified
Hardware:	All All

Importance:	P5 - low enhancement
Assignee:	Tomás Cohen Arazi (tcohen)
QA Contact:	Matt Blenkinsop

URL:
Keywords:	additional_work_needed

Depends on:	37419 37672 37510
Blocks:
	Show dependency tree / graph

Reported:	2024-07-29 15:08 UTC by Tomás Cohen Arazi (tcohen)
Modified:	2024-09-10 08:42 UTC (History)
CC List:	8 users (show)

See Also:
Change sponsored?:	---
Patch complexity:	---
Documentation contact:
Documentation submission:
Text to go in the release notes:	This enhancement to the record sources feature (added in Koha 24.05) removes the 'Delete' action from the record sources table if there are bibliographic records that use the record source. Previously, if you tried to delete a record source that was in use, you would get an unhelpful error message "Something went wrong: Error: Something went wrong, check Koha logs for details.".
Version(s) released in:	24.11.00
Circulation function:

Attachments
Bug 37513: Handle Koha::Exceptions::Object::FKConstraintDeletio in controller (3.09 KB, patch) 2024-08-19 17:58 UTC, Tomás Cohen Arazi (tcohen)	Details \| Diff \| Splinter Review
Bug 37513: Add Koha::RecordSource->usage_count (2.78 KB, patch) 2024-08-19 17:58 UTC, Tomás Cohen Arazi (tcohen)	Details \| Diff \| Splinter Review
Bug 37513: Allow embedding usage_count in record sources (1.62 KB, patch) 2024-08-19 17:58 UTC, Tomás Cohen Arazi (tcohen)	Details \| Diff \| Splinter Review
Bug 37513: Add a way for KohaTable to decide if a button should be displayed (2.48 KB, patch) 2024-08-19 17:58 UTC, Tomás Cohen Arazi (tcohen)	Details \| Diff \| Splinter Review
Bug 37513: Only display delete button if usage_count == 0 (1.62 KB, patch) 2024-08-19 17:58 UTC, Tomás Cohen Arazi (tcohen)	Details \| Diff \| Splinter Review
Bug 37513: Handle Koha::Exceptions::Object::FKConstraintDeletio in controller (3.13 KB, patch) 2024-08-21 05:05 UTC, David Nind	Details \| Diff \| Splinter Review
Bug 37513: Add Koha::RecordSource->usage_count (2.83 KB, patch) 2024-08-21 05:05 UTC, David Nind	Details \| Diff \| Splinter Review
Bug 37513: Allow embedding usage_count in record sources (1.66 KB, patch) 2024-08-21 05:06 UTC, David Nind	Details \| Diff \| Splinter Review
Bug 37513: Add a way for KohaTable to decide if a button should be displayed (2.53 KB, patch) 2024-08-21 05:06 UTC, David Nind	Details \| Diff \| Splinter Review
Bug 37513: Only display delete button if usage_count == 0 (1.67 KB, patch) 2024-08-21 05:06 UTC, David Nind	Details \| Diff \| Splinter Review
Bug 37513: Handle Koha::Exceptions::Object::FKConstraintDeletio in controller (3.22 KB, patch) 2024-08-21 10:08 UTC, Matt Blenkinsop	Details \| Diff \| Splinter Review
Bug 37513: Add Koha::RecordSource->usage_count (2.91 KB, patch) 2024-08-21 10:08 UTC, Matt Blenkinsop	Details \| Diff \| Splinter Review
Bug 37513: Allow embedding usage_count in record sources (1.75 KB, patch) 2024-08-21 10:08 UTC, Matt Blenkinsop	Details \| Diff \| Splinter Review
Bug 37513: Add a way for KohaTable to decide if a button should be displayed (2.61 KB, patch) 2024-08-21 10:08 UTC, Matt Blenkinsop	Details \| Diff \| Splinter Review
Bug 37513: Only display delete button if usage_count == 0 (1.75 KB, patch) 2024-08-21 10:08 UTC, Matt Blenkinsop	Details \| Diff \| Splinter Review
Bug 37513: Fix and add more Cypress tests (5.28 KB, patch) 2024-09-09 15:08 UTC, Tomás Cohen Arazi (tcohen)	Details \| Diff \| Splinter Review
Output from test failure for RecordSources (first tme success - second time failed) (12.77 KB, text/plain) 2024-09-09 21:37 UTC, David Nind	Details
Show Obsolete (10) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Tomás Cohen Arazi (tcohen) 2024-07-29 15:08:10 UTC

With bug 37419 and bug 37510 we should be able to make the UI display a more meaningful message.

Work to be done:

* Add Koha::Exceptions::Object::FKConstraintDeletion handling to the API controller
* Handle this API exception in the Vue component.

Questions:

* Should the exception handling be generic? Ad-hoc on this controller?

Comment 1 Tomás Cohen Arazi (tcohen) 2024-08-19 17:58:04 UTC

Created attachment 170475 [details] [review]
Bug 37513: Handle Koha::Exceptions::Object::FKConstraintDeletio in controller

Comment 2 Tomás Cohen Arazi (tcohen) 2024-08-19 17:58:07 UTC

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

Comment 3 Tomás Cohen Arazi (tcohen) 2024-08-19 17:58:09 UTC

Created attachment 170477 [details] [review]
Bug 37513: Allow embedding usage_count in record sources

Comment 4 Tomás Cohen Arazi (tcohen) 2024-08-19 17:58:12 UTC

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.

Comment 5 Tomás Cohen Arazi (tcohen) 2024-08-19 17:58:14 UTC

Created attachment 170479 [details] [review]
Bug 37513: Only display delete button if usage_count == 0

Comment 6 David Nind 2024-08-21 05:05:55 UTC

Created attachment 170512 [details] [review]
Bug 37513: Handle Koha::Exceptions::Object::FKConstraintDeletio in controller

Signed-off-by: David Nind <david@davidnind.com>

Comment 7 David Nind 2024-08-21 05:05:58 UTC

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>

Comment 8 David Nind 2024-08-21 05:06:00 UTC

Created attachment 170514 [details] [review]
Bug 37513: Allow embedding usage_count in record sources

Signed-off-by: David Nind <david@davidnind.com>

Comment 9 David Nind 2024-08-21 05:06:03 UTC

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>

Comment 10 David Nind 2024-08-21 05:06:06 UTC

Created attachment 170516 [details] [review]
Bug 37513: Only display delete button if usage_count == 0

Signed-off-by: David Nind <david@davidnind.com>

Comment 11 David Nind 2024-08-21 05:21:03 UTC

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

Comment 12 Matt Blenkinsop 2024-08-21 10:08:38 UTC

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>

Comment 13 Matt Blenkinsop 2024-08-21 10:08:40 UTC

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>

Comment 14 Matt Blenkinsop 2024-08-21 10:08:43 UTC

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>

Comment 15 Matt Blenkinsop 2024-08-21 10:08:46 UTC

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>

Comment 16 Matt Blenkinsop 2024-08-21 10:08:48 UTC

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>

Comment 17 Katrin Fischer 2024-08-27 10:02:17 UTC

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)

Comment 18 Katrin Fischer 2024-08-27 10:15:18 UTC

Pushed for 24.11!

Well done everyone, thank you!

Comment 19 Jonathan Druart 2024-09-09 08:14:14 UTC

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.

Comment 20 Tomás Cohen Arazi (tcohen) 2024-09-09 15:08:56 UTC

Created attachment 171205 [details] [review]
Bug 37513: Fix and add more Cypress tests

Comment 21 Katrin Fischer 2024-09-09 15:21:04 UTC

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)

Comment 22 Katrin Fischer 2024-09-09 15:44:40 UTC

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.

Comment 23 David Nind 2024-09-09 21:36:29 UTC

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!

Comment 24 David Nind 2024-09-09 21:37:39 UTC

Created attachment 171236 [details]
Output from test failure for RecordSources (first tme success - second time failed)

Comment 25 Katrin Fischer 2024-09-10 08:41:44 UTC

It started to work for me with the second run and all runs after - this doesn't work for you?

Comment 26 Katrin Fischer 2024-09-10 08:42:12 UTC

Hm. OK - I see it didn't. Back to Tomas.