Attachment 175019 Details for Bug 38547 – Bug 38547: Bring controller in line with modern standards

[patch] Bug 38547: Bring controller in line with modern standards

Bug-38547-Bring-controller-in-line-with-modern-sta.patch (text/plain), 11.39 KB, created by Matt Blenkinsop on 2024-11-28 15:17:00 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Matt Blenkinsop

Created: 2024-11-28 15:17:00 UTC

Size: 11.39 KB

patch

obsolete

>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)

Actions: View | Diff | Splinter Review

Attachments on bug 38547: 175018 | 175019