From 4b2c3e0c8e867ad4e62794cb82e6a3416623cce8 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Mon, 10 May 2021 17:44:23 -0300 Subject: [PATCH] Bug 28272: Fix many things... This patch could've been splitted into several. But, overall, adding additionalAttributes: false made the API fail on requests that send extra info (i.e. cases in which a dev added an attribute to the underlaying class/table and forgot to deal with it on the API (either adding it on the spec, or removing it from the response using Koha::Class::to_api_mapping). - Koha::Account::Line was missing: credit_type, interface, status, register_id and credit_number. I decided to call cash_register_id, and to remove credit_number from the response. FIXME: We need consensus on a name for the credit_number attribute, and add it to the response on the API. It deserves a separate bug. Too opinionated for a last-minute fix. - Koha::Club::Hold::add was returning bad auto-calculated values on field that (also) wasn't specified on the spec. Needs a test. - import_batch_profile had a typo: id_profile vs. profile_id. - error.json: In this case I reverted the change. This is because some routes are adding more 'info' with the error message, and I consider this should be done in a more generic approach. Time is required for us to think about this. So don't break the API in the meantime. FIXME: Implement a generic way to add a payload to error messages on the API. Maybe something to work on while on bug 28020. To test: 1. Run: $ kshell k$ prove t/db_dependent/api/v1/ => FAIL: Lots of tests fail 2. Apply this patch 3. Repeat 1 => SUCCESS: Tests pass! 4. Sign off :-D Signed-off-by: Tomas Cohen Arazi --- Koha/Account/Line.pm | 2 ++ Koha/Club/Hold.pm | 1 + api/v1/swagger/definitions/account_line.json | 19 +++++++++++++++++++ api/v1/swagger/definitions/club_hold.json | 5 +++++ api/v1/swagger/definitions/error.json | 3 +-- .../definitions/import_batch_profile.json | 2 +- t/db_dependent/api/v1/import_batch_profiles.t | 3 +-- 7 files changed, 30 insertions(+), 5 deletions(-) diff --git a/Koha/Account/Line.pm b/Koha/Account/Line.pm index 566177d4b7e..9418267625e 100644 --- a/Koha/Account/Line.pm +++ b/Koha/Account/Line.pm @@ -943,6 +943,7 @@ on the API. sub to_api_mapping { return { accountlines_id => 'account_line_id', + credit_number => undef, credit_type_code => 'credit_type', debit_type_code => 'debit_type', amountoutstanding => 'amount_outstanding', @@ -952,6 +953,7 @@ sub to_api_mapping { itemnumber => 'item_id', manager_id => 'user_id', note => 'internal_note', + register_id => 'cash_register_id', }; } diff --git a/Koha/Club/Hold.pm b/Koha/Club/Hold.pm index 378cd027f71..46f9f319330 100644 --- a/Koha/Club/Hold.pm +++ b/Koha/Club/Hold.pm @@ -79,6 +79,7 @@ sub add { }; my $club_hold = Koha::Club::Hold->new($club_params)->store(); + $club_hold->discard_changes; @enrollments = shuffle(@enrollments); diff --git a/api/v1/swagger/definitions/account_line.json b/api/v1/swagger/definitions/account_line.json index ddf901602cc..9fc85b66871 100644 --- a/api/v1/swagger/definitions/account_line.json +++ b/api/v1/swagger/definitions/account_line.json @@ -53,6 +53,13 @@ ], "description": "Account line debit type" }, + "credit_type": { + "type": [ + "string", + "null" + ], + "description": "Account line credit type" + }, "payment_type": { "type": [ "string", @@ -96,6 +103,18 @@ "null" ], "description": "Internal identifier for the library in which the transaction took place" + }, + "interface": { + "type": [ "string", "null" ], + "description": "Interface in which the account line was generated (values can be: api, cron, commandline, intranet, opac and sip)" + }, + "status": { + "type": [ "string", "null" ], + "description": "The credit/debit status" + }, + "cash_register_id": { + "type": [ "integer", "null" ], + "description": "Internal identifier for the cash register used for the payment (if any)" } }, "additionalProperties": false diff --git a/api/v1/swagger/definitions/club_hold.json b/api/v1/swagger/definitions/club_hold.json index 68cede65a1d..538a1c68536 100644 --- a/api/v1/swagger/definitions/club_hold.json +++ b/api/v1/swagger/definitions/club_hold.json @@ -16,6 +16,11 @@ "item_id": { "type": ["string", "null"], "description": "Internal item identifier" + }, + "date_created": { + "type": "string", + "format": "date-time", + "description": "Date and time the hold was created" } }, "additionalProperties": false diff --git a/api/v1/swagger/definitions/error.json b/api/v1/swagger/definitions/error.json index b0fe4fc9929..e43673f2174 100644 --- a/api/v1/swagger/definitions/error.json +++ b/api/v1/swagger/definitions/error.json @@ -5,6 +5,5 @@ "description": "Error message", "type": "string" } - }, - "additionalProperties": false + } } diff --git a/api/v1/swagger/definitions/import_batch_profile.json b/api/v1/swagger/definitions/import_batch_profile.json index 98e3aa85c79..004efec31bb 100644 --- a/api/v1/swagger/definitions/import_batch_profile.json +++ b/api/v1/swagger/definitions/import_batch_profile.json @@ -1,7 +1,7 @@ { "type": "object", "properties": { - "id_profile": { + "profile_id": { "type": "integer", "description": "Internal profile identifier" }, diff --git a/t/db_dependent/api/v1/import_batch_profiles.t b/t/db_dependent/api/v1/import_batch_profiles.t index 817067b37ab..e0eb6adcb8a 100755 --- a/t/db_dependent/api/v1/import_batch_profiles.t +++ b/t/db_dependent/api/v1/import_batch_profiles.t @@ -106,7 +106,6 @@ subtest 'list profiles' => sub { ->json_is('/0/name', $ibp1->name) ->json_is('/1/name', $ibp2->name); - $schema->storage->txn_rollback; }; @@ -238,4 +237,4 @@ subtest 'delete profile' => sub { $schema->storage->txn_rollback; -}; \ No newline at end of file +}; -- 2.31.1