From 399804a0d3c30b55698616e307d4c35043cacfe3 Mon Sep 17 00:00:00 2001
From: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Date: Thu, 28 Nov 2024 15:12:27 +0000
Subject: [PATCH] Bug 38547: Bring controller in line with modern standards

Test plan:
1) In system preferences, click Search and then select the Acquisitions option from the left hand menu
2) Enable MarcOrderingAutomation
3) In the administration homepage, under Acquisition parameters there should now be a link called "MARC order accounts"
4) Click on this and click the New account button
5) Create a new account and then experiment with editing and deleting the account.
6) These actions should all work and there should be bootstrap messages displayed for each action
---
 Koha/ImportBatch.pm                           |  3 +-
 Koha/Schema/Result/MarcOrderAccount.pm        |  9 +++
 admin/marc_order_accounts.pl                  | 50 +++++++++++-----
 .../en/modules/admin/marc_order_accounts.tt   | 60 +++++++++++++++----
 4 files changed, 94 insertions(+), 28 deletions(-)

diff --git a/Koha/ImportBatch.pm b/Koha/ImportBatch.pm
index 3f63df6e564..43c6c5744db 100644
--- a/Koha/ImportBatch.pm
+++ b/Koha/ImportBatch.pm
@@ -75,7 +75,6 @@ sub new_from_file {
     my $basket_id                  = $args->{basket_id};
     my $profile_id                 = $args->{profile_id};
 
-    my @messages;
     my ( $batch_id, $num_valid, $num_items, @import_errors );
     my $num_with_matches = 0;
     my $checked_matches  = 0;
@@ -172,7 +171,7 @@ sub new_from_file {
         basket_id       => $basket_id,
     };
 
-    return { report => $report, messages => \@messages };
+    return { report => $report };
 }
 
 
diff --git a/Koha/Schema/Result/MarcOrderAccount.pm b/Koha/Schema/Result/MarcOrderAccount.pm
index 0595d9e3f15..fd8f6d67a01 100644
--- a/Koha/Schema/Result/MarcOrderAccount.pm
+++ b/Koha/Schema/Result/MarcOrderAccount.pm
@@ -227,5 +227,14 @@ __PACKAGE__->add_columns(
     '+parse_items' => { is_boolean => 1 },
 );
 
+sub koha_object_class {
+    'Koha::Acquisition::MarcOrderAccount';
+}
+
+sub koha_objects_class {
+    'Koha::Acquisition::MarcOrderAccounts';
+}
+
+
 # You can replace this text with custom code or comments, and it will be preserved on regeneration
 1;
diff --git a/admin/marc_order_accounts.pl b/admin/marc_order_accounts.pl
index 7f2bcae0640..94597375b7f 100755
--- a/admin/marc_order_accounts.pl
+++ b/admin/marc_order_accounts.pl
@@ -48,22 +48,21 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
     }
 );
 
-my $crypt = Koha::Encryption->new;
-
 my $op = $input->param('op');
-$op ||= 'display';
+$op ||= 'list';
+my @messages;
 
-if ( $op eq 'acct_form' ) {
-    $template->param( acct_form => 1 );
+if ( $op eq 'add_form' ) {
+    $op = 'add_form';
     my $budgets = GetBudgets();
     $template->param( budgets => $budgets );
     my @matchers = C4::Matcher::GetMatcherList();
     $template->param( available_matchers => \@matchers );
 
     show_account( $input, $template );
-} elsif ( $op eq 'delete_acct' ) {
+} elsif ( $op eq 'delete_confirm' ) {
     show_account( $input, $template );
-    $template->param( delete_acct => 1 );
+    $op = 'delete_confirm';
 } else {
     if ( $op eq 'cud-save' ) {
         my $fields = {
@@ -86,22 +85,41 @@ if ( $op eq 'acct_form' ) {
         if ( scalar $input->param('id') ) {
 
             # Update existing account
-            my $account = Koha::MarcOrderAccounts->find( scalar $input->param('id') );
-            $account->update($fields);
+            my $account = Koha::Acquisition::MarcOrderAccounts->find( scalar $input->param('id') );
+            eval { $account->update($fields); };
+            if ($@) {
+                push @messages, { type => 'error', code => 'error_on_update' };
+            } else {
+                push @messages, { type => 'message', code => 'success_on_update' };
+            }
+
         } else {
 
             # Add new account
-            my $new_account = Koha::MarcOrderAccount->new($fields);
-            $new_account->store;
+            my $new_account = Koha::Acquisition::MarcOrderAccount->new($fields);
+            eval { $new_account->store; };
+            if ($@) {
+                push @messages, { type => 'error', code => 'error_on_insert' };
+            } else {
+                push @messages, { type => 'message', code => 'success_on_insert' };
+            }
+
         }
-    } elsif ( $op eq 'cud-delete_acct' ) {
+        $op = 'list';
+    } elsif ( $op eq 'cud-delete_confirmed' ) {
         my $acct_id = $input->param('id');
-        my $acct    = Koha::MarcOrderAccounts->find($acct_id);
-        $acct->delete;
+        my $acct    = Koha::Acquisition::MarcOrderAccounts->find($acct_id);
+        my $deleted = eval { $acct->delete; };
+
+        if ( $@ or not $deleted ) {
+            push @messages, { type => 'error', code => 'error_on_delete' };
+        } else {
+            push @messages, { type => 'message', code => 'success_on_delete' };
+        }
+        $op = 'list';
     }
 
-    $template->param( display => 1 );
-    my @accounts = Koha::MarcOrderAccounts->search(
+    my @accounts = Koha::Acquisition::MarcOrderAccounts->search(
         {},
         { join => [ 'vendor', 'budget' ] }
     )->as_list;
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_order_accounts.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_order_accounts.tt
index 3ef12485b3a..d63645fce11 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_order_accounts.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc_order_accounts.tt
@@ -24,15 +24,31 @@ MARC order accounts
             [% END %]
         [% END %]
 
-        [% IF acct_form || delete_confirm %]
+        [% IF op == 'add_form' %]
             [% WRAPPER breadcrumb_item %]
                 <a href="/cgi-bin/koha/admin/marc_order_accounts.pl">MARC order accounts</a>
             [% END %]
+            [% WRAPPER breadcrumb_item bc_active = 1 %]
+                [% IF account.id %]
+                    [% tx("Modify account '{account}'", { account = account.id }) | html %]
+                [% ELSE %]
+                    <span>New account</span>
+                [% END %]
+            [% END %]
+
+        [% ELSIF op == 'delete_confirm' %]
+            [% WRAPPER breadcrumb_item %]
+                <a href="/cgi-bin/koha/admin/marc_order_accounts.pl">MARC order accounts</a>
+            [% END %]
+            [% WRAPPER breadcrumb_item bc_active = 1 %]
+                [% tx("Confirm deletion of account '{account}'", { account = account.id }) | html %]
+            [% END %]
+
         [% ELSE %]
-            [% WRAPPER breadcrumb_item bc_active= 1 %]
+            [% WRAPPER breadcrumb_item bc_active = 1 %]
                 <span>MARC order accounts</span>
             [% END %]
-        [% END %]
+        [% END #/IF op = add_form %]
     [% END #/ WRAPPER breadcrumbs %]
 [% END #/ WRAPPER sub-header.inc %]
 
@@ -41,9 +57,33 @@ MARC order accounts
         <div class="col-md-10 order-md-2 order-sm-1">
             <main>
                 [% INCLUDE 'messages.inc' %]
-                [% IF display %]
+
+                [% FOR m IN messages %]
+                    <div class="alert alert-[% m.type | html %]">
+                        [% SWITCH m.code %]
+                        [% CASE 'error_on_update' %]
+                            <span>An error occurred when updating this account. Perhaps it already exists.</span>
+                        [% CASE 'error_on_insert' %]
+                            <span>An error occurred when adding this account. The account ID might already exist.</span>
+                        [% CASE 'error_on_delete' %]
+                            <span>An error occurred when deleting this account. Check the logs for details.</span>
+                        [% CASE 'success_on_update' %]
+                            <span>Account updated successfully.</span>
+                        [% CASE 'success_on_insert' %]
+                            <span>Account added successfully.</span>
+                        [% CASE 'success_on_delete' %]
+                            <span>Account deleted successfully.</span>
+                        [% CASE 'already_exists' %]
+                            <span>This account already exists.</span>
+                        [% CASE %]
+                            <span>[% m.code | html %]</span>
+                        [% END %]
+                    </div>
+                [% END %]
+
+                [% IF op == 'list' %]
                     <div id="toolbar" class="btn-toolbar">
-                    <a class="btn btn-default" id="newmarcorderacct" href="/cgi-bin/koha/admin/marc_order_accounts.pl?op=acct_form">
+                    <a class="btn btn-default" id="newmarcorderacct" href="/cgi-bin/koha/admin/marc_order_accounts.pl?op=add_form">
                         <i class="fa fa-plus"></i>
                         New account
                     </a>
@@ -68,8 +108,8 @@ MARC order accounts
                                         <td>[% account.description | html %]</td>
                                         <td>[% account.download_directory | html %]</td>
                                         <td class="actions">
-                                            <a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/marc_order_accounts.pl?op=acct_form&id=[% account.id | html %]"><i class="fa fa-pencil-alt"></i> Edit</a>
-                                            <a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/marc_order_accounts.pl?op=delete_acct&id=[% account.id | html %]"><i class="fa fa-trash-can"></i> Delete</a>
+                                            <a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/marc_order_accounts.pl?op=add_form&id=[% account.id | html %]"><i class="fa fa-pencil-alt"></i> Edit</a>
+                                            <a class="btn btn-default btn-xs" href="/cgi-bin/koha/admin/marc_order_accounts.pl?op=delete_confirm&id=[% account.id | html %]"><i class="fa fa-trash-can"></i> Delete</a>
                                         </td>
                                     </tr>
                                 [% END %]
@@ -81,7 +121,7 @@ MARC order accounts
                         </div>
                     [% END %]
                 [% END %]
-                [% IF acct_form %]
+                [% IF op == 'add_form' %]
                     <h1>
                         [% IF account %]
                             Modify account
@@ -261,7 +301,7 @@ MARC order accounts
                         </fieldset>
                     </form>
                 [% END %]
-                [% IF delete_acct %]
+                [% IF op == 'delete_confirm' %]
                     <div class="dialog alert">
                         <h1>Delete this account?</h1>
                         <table>
@@ -277,7 +317,7 @@ MARC order accounts
                         </table>
                         <form action="/cgi-bin/koha/admin/marc_order_accounts.pl" method="post">
                             [% INCLUDE 'csrf-token.inc' %]
-                            <input type="hidden" name="op" value="cud-delete_acct" />
+                            <input type="hidden" name="op" value="cud-delete_confirmed" />
                             <input type="hidden" name="id" value="[% account.id | html %]" />
                             <button type="submit" class="approve"><i class="fa fa-fw fa-check"></i> Yes, delete</button>
                         </form>
-- 
2.39.3 (Apple Git-146)