From 27f610de8f1c7c1d3d8d50ad524c09ec2cdd4e36 Mon Sep 17 00:00:00 2001 From: Erik Huelsmann Date: Fri, 24 Jan 2025 22:18:12 +0100 Subject: [PATCH 01/12] Prepare removal of core entities Users want to be able to (reliably) remove accounts, headings, companies, persons and entity_credit_accounts (customers/vendors). --- UI/Contact/divs/company.html | 27 ++-- lib/LedgerSMB/Entity/Company.pm | 11 +- .../1.13/fk-actions-for-is_used-functions.sql | 122 ++++++++++++++++++ sql/changes/LOADORDER | 1 + sql/modules/Account.sql | 55 ++++++-- sql/modules/BLACKLIST | 4 + sql/modules/Company.sql | 37 +++++- sql/modules/Entity.sql | 24 ++++ 8 files changed, 256 insertions(+), 25 deletions(-) create mode 100644 sql/changes/1.13/fk-actions-for-is_used-functions.sql diff --git a/UI/Contact/divs/company.html b/UI/Contact/divs/company.html index 73dc850602..60be421d16 100644 --- a/UI/Contact/divs/company.html +++ b/UI/Contact/divs/company.html @@ -133,14 +133,25 @@ value = 'get_by_cc' class = 'submit' } %] - [% INCLUDE button element_data = { - text = text('Save') - id = 'company-action-save' - class="submit" - type="submit" - name="__action" - value="save_company" - } %] + [% INCLUDE button element_data = { + text = text('Save') + id = 'company-action-save' + class="submit" + type="submit" + name="__action" + value="save_company" + } %] + [% + IF entity_id.defined AND NOT company.is_used ; + INCLUDE button element_data = { + text = text('Delete') + id = 'company-action-delete' + class="submit" + type="submit" + name="__action" + value="delete_company" + }; + END %] [% FOREACH n = notes %] diff --git a/lib/LedgerSMB/Entity/Company.pm b/lib/LedgerSMB/Entity/Company.pm index 57b4d31fe5..204fdb9da2 100644 --- a/lib/LedgerSMB/Entity/Company.pm +++ b/lib/LedgerSMB/Entity/Company.pm @@ -81,11 +81,20 @@ has 'sic_code' => (is => 'rw', isa => 'Maybe[Str]', required => 0); Date when the company was entered into LedgerSMB +=cut + +has 'created' => (is => 'rw', isa => 'LedgerSMB::PGDate'); + +=item is_used + +Boolean (Readonly). True if the company entity is used in the database +(and hence can't be deleted). + =back =cut -has 'created' => (is => 'rw', isa => 'LedgerSMB::PGDate'); +has 'is_used' => (is => 'ro'); =head1 METHODS diff --git a/sql/changes/1.13/fk-actions-for-is_used-functions.sql b/sql/changes/1.13/fk-actions-for-is_used-functions.sql new file mode 100644 index 0000000000..ca58e730bd --- /dev/null +++ b/sql/changes/1.13/fk-actions-for-is_used-functions.sql @@ -0,0 +1,122 @@ + +/* + + This file contains the revisited foreign key constraints which need ON DELETE + set to CASCADE, instead of 'NO ACTION', in order to make the *__is_used() + functions work. These functions try to delete a record (e.g., an account); + when that succeeds, it concludes that the account isn't used (and aborts the + deletion!). + + However, accounts will have associated account_link records and + cr_coa_to_account records. These should not block deletion however, meaning + that the delete should cascade. This way, the delete gets blocked when the + deletion of the cr_coa_to_account row deletion fails due to further foreign + keys being checked (and failing). + + */ + + +-- ACCOUNT (gl accounts) +ALTER TABLE account_link + DROP CONSTRAINT account_link_account_id_fkey, + ADD FOREIGN KEY (account_id) REFERENCES account (id) ON DELETE CASCADE; + +ALTER TABLE cr_coa_to_account + DROP CONSTRAINT cr_coa_to_account_chart_id_fkey, + ADD FOREIGN KEY (chart_id) REFERENCES account (id) ON DELETE CASCADE; + +-- ENTITY + +-- entity additional data +ALTER TABLE entity_other_name + DROP CONSTRAINT entity_other_name_entity_id_fkey, + ADD FOREIGN KEY (entity_id) REFERENCES entity (id) ON DELETE CASCADE; + +ALTER TABLE entity_to_contact + DROP CONSTRAINT entity_to_contact_entity_id_fkey, + ADD FOREIGN KEY (entity_id) REFERENCES entity (id) ON DELETE CASCADE; + +ALTER TABLE entity_to_location + DROP CONSTRAINT entity_to_location_entity_id_fkey, + ADD FOREIGN KEY (entity_id) REFERENCES entity (id) ON DELETE CASCADE; + +ALTER TABLE entity_bank_account + DROP CONSTRAINT entity_bank_account_entity_id_fkey, + ADD FOREIGN KEY (entity_id) REFERENCES entity (id) ON DELETE CASCADE; + +ALTER TABLE entity_note + DROP CONSTRAINT entity_note_entity_id_fkey, + ADD FOREIGN KEY (entity_id) REFERENCES entity (id) ON DELETE CASCADE; + +ALTER TABLE entity_note + DROP CONSTRAINT entity_note_ref_key_fkey, + ADD FOREIGN KEY (ref_key) REFERENCES entity (id) ON DELETE CASCADE; + +ALTER TABLE file_entity + DROP CONSTRAINT file_entity_ref_key_fkey, + ADD FOREIGN KEY (ref_key) REFERENCES entity (id) ON DELETE CASCADE; + +-- entity type specialization +ALTER TABLE company + DROP CONSTRAINT company_entity_id_fkey, + ADD FOREIGN KEY (entity_id) REFERENCES entity (id) ON DELETE CASCADE; + +ALTER TABLE person + DROP CONSTRAINT person_entity_id_fkey, + ADD FOREIGN KEY (entity_id) REFERENCES entity (id) ON DELETE CASCADE; + +ALTER TABLE robot + DROP CONSTRAINT robot_entity_id_fkey, + ADD FOREIGN KEY (entity_id) REFERENCES entity (id) ON DELETE CASCADE; + +-- entity roles +ALTER TABLE entity_credit_account + DROP CONSTRAINT entity_credit_account_entity_id_fkey, + ADD FOREIGN KEY (entity_id) REFERENCES entity (id) ON DELETE CASCADE; + +ALTER TABLE entity_employee + DROP CONSTRAINT entity_employee_entity_id_fkey, + ADD FOREIGN KEY (entity_id) REFERENCES entity (id) ON DELETE CASCADE; + +-- explicitly not adding 'users': cascading will delete the user +-- but whether users are used or not isn't detectable in the schema + + +-- ENTITY_CREDIT_ACCOUNT + +-- eca additional data + +ALTER TABLE eca_to_contact + DROP CONSTRAINT eca_to_contact_credit_id_fkey, + ADD FOREIGN KEY (credit_id) REFERENCES entity_credit_account (id) ON DELETE CASCADE; + +ALTER TABLE eca_to_location + DROP CONSTRAINT eca_to_location_credit_id_fkey, + ADD FOREIGN KEY (credit_id) REFERENCES entity_credit_account (id) ON DELETE CASCADE; + +ALTER TABLE eca_note + DROP CONSTRAINT eca_note_ref_key_fkey, + ADD FOREIGN KEY (ref_key) REFERENCES entity_credit_account (id) ON DELETE CASCADE; + +ALTER TABLE eca_tax + DROP CONSTRAINT eca_tax_eca_id_fkey, + ADD FOREIGN KEY (eca_id) REFERENCES entity_credit_account (id) ON DELETE CASCADE; + +ALTER TABLE file_eca + DROP CONSTRAINT file_eca_ref_key_fkey, + ADD FOREIGN KEY (ref_key) REFERENCES entity_credit_account (id) ON DELETE CASCADE; + +-- (price matrix data as customer) +ALTER TABLE partscustomer + DROP CONSTRAINT partscustomer_credit_id_fkey, + ADD FOREIGN KEY (credit_id) REFERENCES entity_credit_account (id) ON DELETE CASCADE; + +-- (price matrix data as vendor) +ALTER TABLE partsvendor + DROP CONSTRAINT partsvendor_credit_id_fkey, + ADD FOREIGN KEY (credit_id) REFERENCES entity_credit_account (id) ON DELETE CASCADE; + +-- eca as business reporting unit +ALTER TABLE business_unit + DROP CONSTRAINT business_unit_credit_id_fkey, + ADD FOREIGN KEY (credit_id) REFERENCES entity_credit_account (id) ON DELETE CASCADE; diff --git a/sql/changes/LOADORDER b/sql/changes/LOADORDER index fd972dc79c..39efadbf04 100644 --- a/sql/changes/LOADORDER +++ b/sql/changes/LOADORDER @@ -196,3 +196,4 @@ mc/delete-migration-validation-data.sql # 1.13 changes 1.13/change-logout.sql 1.13/rm-status.sql +1.13/fk-actions-for-is_used-functions.sql diff --git a/sql/modules/Account.sql b/sql/modules/Account.sql index 81bba3fa00..1d2f22ce3a 100644 --- a/sql/modules/Account.sql +++ b/sql/modules/Account.sql @@ -228,6 +228,50 @@ COMMENT ON FUNCTION account__get_from_accno(in_accno text) IS $$ Returns the account where the accno field matches (excatly) the in_accno provided.$$; +CREATE OR REPLACE FUNCTION account__is_used(in_id int) +RETURNS BOOL AS +$$ +BEGIN + BEGIN + delete from account where id = in_id; + raise sqlstate 'P0004'; + EXCEPTION + WHEN foreign_key_violation THEN + return true; + WHEN assert_failure THEN + return false; + END; +END; +$$ language plpgsql; + +COMMENT ON FUNCTION account__is_used(in_id int) IS + $$Checks whether the general ledger account is used or not. + +In case it isn't used, it should be possible to delete it. +$$; --' + +CREATE OR REPLACE FUNCTION account_heading__is_used(in_id int) +RETURNS BOOL AS +$$ +BEGIN + BEGIN + delete from account_heading where id = in_id; + raise sqlstate 'P0004'; + EXCEPTION + WHEN foreign_key_violation THEN + return true; + WHEN assert_failure THEN + return false; + END; +END; +$$ language plpgsql; + +COMMENT ON FUNCTION account_heading__is_used(in_id int) IS + $$Checks whether the general ledger account heading is used or not. + +In case it isn't used, it should be possible to delete it. +$$; --' + CREATE OR REPLACE FUNCTION account__is_recon(in_accno text) RETURNS BOOL AS $$ SELECT count(*) > 0 FROM cr_coa_to_account c2a @@ -747,21 +791,14 @@ ta(account_id) AS ( SELECT a.id, false, a.accno, a.description, a.category, a.contra, (EXISTS (select 1 from cr_coa_to_account cca where chart_id = a.id)), - a.tax, a.obsolete, a.gifi_accno, - (EXISTS (select 1 from ac where a.id = ac.chart_id) - OR EXISTS (select 1 from eca where a.id = eca.account_id) - OR EXISTS (select 1 from p where a.id = p.account_id) - OR EXISTS (select 1 from ta where a.id = ta.account_id) - ), + a.tax, a.obsolete, a.gifi_accno, account__is_used(a.id), link FROM account a LEFT JOIN l ON a.id = l.account_id UNION SELECT id, true, accno, description, null::char(1), null::boolean, null::boolean, null::boolean, null::boolean, null::text, - (EXISTS (select 1 from ha where h.id = ha.heading) - OR EXISTS (select 1 from hh where h.id = hh.parent_id)), - null::text + account_heading__is_used(h.id), null::text FROM account_heading h ) x ORDER BY accno; diff --git a/sql/modules/BLACKLIST b/sql/modules/BLACKLIST index 2544e9ad38..d7750c1d21 100644 --- a/sql/modules/BLACKLIST +++ b/sql/modules/BLACKLIST @@ -6,6 +6,7 @@ account__get_by_link_desc account__get_from_accno account__get_taxes account__is_recon +account__is_used account__list_by_heading account__list_translations account__obtain_balance @@ -18,6 +19,7 @@ account_has_transactions account_heading__check_tree account_heading__delete account_heading__delete_translation +account_heading__is_used account_heading__list account_heading__list_translations account_heading__save_translation @@ -184,6 +186,7 @@ eca__get_pricematrix_by_pricegroup eca__get_taxes eca__history eca__history_summary +eca__is_used eca__list_contacts eca__list_locations eca__list_notes @@ -207,6 +210,7 @@ entity__delete_contact entity__delete_location entity__get entity__get_bank_account +entity__is_used entity__list_bank_account entity__list_classes entity__list_contacts diff --git a/sql/modules/Company.sql b/sql/modules/Company.sql index 292b0133d1..7d909c8023 100644 --- a/sql/modules/Company.sql +++ b/sql/modules/Company.sql @@ -20,7 +20,8 @@ CREATE TYPE company_entity AS( license_number text, sic_code varchar, control_code text, - country_id int + country_id int, + is_used bool ); DROP TYPE IF EXISTS eca__pricematrix CASCADE; @@ -415,7 +416,27 @@ USING in_entity_class, in_contact, in_contact_info, in_meta_number, END $$ LANGUAGE PLPGSQL; +CREATE OR REPLACE FUNCTION eca__is_used(in_id int) + RETURNS boolean AS +$$ + BEGIN + BEGIN + delete from entity_credit_account where id = in_id; + raise sqlstate 'P0004'; + EXCEPTION + WHEN foreign_key_violation THEN + return true; + WHEN assert_failure THEN + return false; + END; + END; +$$ language plpgsql; +COMMENT ON FUNCTION eca__is_used(in_id int) IS + $$Checks whether the credit account is used or not. + +In case it isn't used, it should be possible to delete it. +$$; --' DROP FUNCTION IF EXISTS eca__get_taxes(in_credit_id int); @@ -618,11 +639,12 @@ entity class.$$; CREATE OR REPLACE FUNCTION company__get (in_entity_id int) RETURNS company_entity AS $$ - SELECT c.entity_id, c.legal_name, c.tax_id, c.sales_tax_id, - c.license_number, c.sic_code, e.control_code, e.country_id - FROM company c - JOIN entity e ON e.id = c.entity_id - WHERE entity_id = $1; + SELECT c.entity_id, c.legal_name, c.tax_id, c.sales_tax_id, + c.license_number, c.sic_code, e.control_code, e.country_id, + entity__is_used(in_entity_id) + FROM company c + JOIN entity e ON e.id = c.entity_id + WHERE entity_id = $1; $$ language sql; COMMENT ON FUNCTION company__get (in_entity_id int) IS @@ -632,7 +654,8 @@ CREATE OR REPLACE FUNCTION company__get_by_cc (in_control_code text) RETURNS company_entity AS $$ SELECT c.entity_id, c.legal_name, c.tax_id, c.sales_tax_id, - c.license_number, c.sic_code, e.control_code, e.country_id + c.license_number, c.sic_code, e.control_code, e.country_id, + entity__is_used(e.id) FROM company c JOIN entity e ON e.id = c.entity_id WHERE e.control_code = $1; diff --git a/sql/modules/Entity.sql b/sql/modules/Entity.sql index 3472d7d65c..c407878adb 100644 --- a/sql/modules/Entity.sql +++ b/sql/modules/Entity.sql @@ -11,6 +11,30 @@ set client_min_messages = 'warning'; BEGIN; + +CREATE OR REPLACE FUNCTION entity__is_used(in_id int) + RETURNS boolean AS + $$ +BEGIN + BEGIN + delete from entity where id = in_id; + raise sqlstate 'P0004'; + EXCEPTION + WHEN foreign_key_violation THEN + return true; + WHEN assert_failure THEN + return false; + END; +END; +$$ language plpgsql; + +COMMENT ON FUNCTION entity__is_used(in_id int) IS + $$Checks whether the entity is used or not. + +In case the entity isn't used, it should be possible to delete it. +$$; --' + + CREATE OR REPLACE FUNCTION entity_save( in_entity_id int, in_name text, in_entity_class INT ) RETURNS INT AS $$ From 596565250cabd94f6af17c296b087c00bee21df8 Mon Sep 17 00:00:00 2001 From: Erik Huelsmann Date: Fri, 24 Jan 2025 22:25:39 +0100 Subject: [PATCH 02/12] Make currency__is_used() robust against schema changes --- sql/modules/Currency.sql | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/sql/modules/Currency.sql b/sql/modules/Currency.sql index bc2dae0a93..fe73deb004 100644 --- a/sql/modules/Currency.sql +++ b/sql/modules/Currency.sql @@ -48,22 +48,18 @@ $$ language sql; COMMENT ON FUNCTION currency__get(text) IS $$Retrieves a currency and its description using the currency indicator.$$; -CREATE OR REPLACE FUNCTION currency__is_used -(in_curr text) +CREATE OR REPLACE FUNCTION currency__is_used(in_curr text) RETURNS BOOLEAN AS $$ BEGIN - RETURN EXISTS (SELECT 1 FROM acc_trans WHERE curr = in_curr) - OR EXISTS (SELECT 1 FROM account_checkpoint WHERE curr = in_curr) - OR EXISTS (SELECT 1 FROM ap WHERE curr = in_curr) - OR EXISTS (SELECT 1 FROM ar WHERE curr = in_curr) - OR EXISTS (SELECT 1 FROM budget_line WHERE curr = in_curr) - OR EXISTS (SELECT 1 FROM entity_credit_account WHERE curr = in_curr) - OR EXISTS (SELECT 1 FROM exchangerate_default WHERE curr = in_curr) - OR EXISTS (SELECT 1 FROM jcitems WHERE curr = in_curr) - OR EXISTS (SELECT 1 FROM journal_line WHERE curr = in_curr) - OR EXISTS (SELECT 1 FROM oe WHERE curr = in_curr) - OR EXISTS (SELECT 1 FROM partscustomer WHERE curr = in_curr) - OR EXISTS (SELECT 1 FROM partsvendor WHERE curr = in_curr); + BEGIN + delete from currency where curr = in_curr; + raise sqlstate 'P0004'; -- cause rollback + EXCEPTION + WHEN foreign_key_violation THEN + return true; + WHEN assert_failure THEN + return false; + END; END;$$ language plpgsql security definer; COMMENT ON FUNCTION currency__is_used(text) IS From 976cb99e8c392b4dbb5011485d75cd14223271c0 Mon Sep 17 00:00:00 2001 From: Erik Huelsmann Date: Fri, 24 Jan 2025 22:47:03 +0100 Subject: [PATCH 03/12] Add 'Delete' button to Contacts > Person page --- UI/Contact/divs/company.html | 2 +- UI/Contact/divs/person.html | 23 +++++++++++++++++------ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/UI/Contact/divs/company.html b/UI/Contact/divs/company.html index 60be421d16..922f3bf096 100644 --- a/UI/Contact/divs/company.html +++ b/UI/Contact/divs/company.html @@ -149,7 +149,7 @@ class="submit" type="submit" name="__action" - value="delete_company" + value="delete_entity" }; END %] diff --git a/UI/Contact/divs/person.html b/UI/Contact/divs/person.html index 51a76e63bf..03d1cafd24 100644 --- a/UI/Contact/divs/person.html +++ b/UI/Contact/divs/person.html @@ -141,13 +141,24 @@ value = 'get_by_cc' class = 'submit' } %] - [% INCLUDE button element_data = { - text = text('Save'), - class="submit" - type="submit" - name="__action" - value="save_person" + [% INCLUDE button element_data = { + text = text('Save'), + class="submit" + type="submit" + name="__action" + value="save_person" } %] + [% + IF entity_id.defined AND NOT company.is_used ; + INCLUDE button element_data = { + text = text('Delete') + id = 'person-action-delete' + class="submit" + type="submit" + name="__action" + value="delete_entity" + }; + END %] [% FOREACH n = notes %]
From a9120c398a97c82f05b70da378c4d149788fab93 Mon Sep 17 00:00:00 2001 From: Erik Huelsmann Date: Fri, 24 Jan 2025 22:48:08 +0100 Subject: [PATCH 04/12] Make 'is_used' attribute common for all entities --- lib/LedgerSMB/Entity.pm | 9 +++++++++ lib/LedgerSMB/Entity/Company.pm | 9 --------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/LedgerSMB/Entity.pm b/lib/LedgerSMB/Entity.pm index 4a343e2b36..d1e26baeb8 100644 --- a/lib/LedgerSMB/Entity.pm +++ b/lib/LedgerSMB/Entity.pm @@ -67,6 +67,15 @@ Name of country (optional) has 'country_name' => (is => 'rw', isa => 'Str', required => 0); +=item is_used + +Boolean (Readonly). True if the company entity is used in the database +(and hence can't be deleted). + +=cut + +has 'is_used' => (is => 'ro'); + =back =head1 METHODS diff --git a/lib/LedgerSMB/Entity/Company.pm b/lib/LedgerSMB/Entity/Company.pm index 204fdb9da2..14bcdd1d08 100644 --- a/lib/LedgerSMB/Entity/Company.pm +++ b/lib/LedgerSMB/Entity/Company.pm @@ -85,17 +85,8 @@ Date when the company was entered into LedgerSMB has 'created' => (is => 'rw', isa => 'LedgerSMB::PGDate'); -=item is_used - -Boolean (Readonly). True if the company entity is used in the database -(and hence can't be deleted). - =back -=cut - -has 'is_used' => (is => 'ro'); - =head1 METHODS =over From 3bb6acc781b7755f9dbef7453874da3eb22f4efe Mon Sep 17 00:00:00 2001 From: Erik Huelsmann Date: Sat, 25 Jan 2025 22:38:10 +0100 Subject: [PATCH 05/12] Implement backend for the Company/Person 'Delete' button --- UI/Contact/divs/company.html | 11 +---------- UI/Contact/divs/person.html | 2 +- lib/LedgerSMB/Entity.pm | 12 ++++++++++++ lib/LedgerSMB/Scripts/contact.pm | 23 ++++++++++++++++++++++- sql/modules/BLACKLIST | 1 + sql/modules/Entity.sql | 19 +++++++++++++++++++ sql/modules/Roles.sql | 1 + 7 files changed, 57 insertions(+), 12 deletions(-) diff --git a/UI/Contact/divs/company.html b/UI/Contact/divs/company.html index 922f3bf096..f5b1b6dc4b 100644 --- a/UI/Contact/divs/company.html +++ b/UI/Contact/divs/company.html @@ -133,16 +133,7 @@ value = 'get_by_cc' class = 'submit' } %] - [% INCLUDE button element_data = { - text = text('Save') - id = 'company-action-save' - class="submit" - type="submit" - name="__action" - value="save_company" - } %] - [% - IF entity_id.defined AND NOT company.is_used ; + IF may_delete AND entity_id.defined AND NOT company.is_used ; INCLUDE button element_data = { text = text('Delete') id = 'company-action-delete' diff --git a/UI/Contact/divs/person.html b/UI/Contact/divs/person.html index 03d1cafd24..21322f76c4 100644 --- a/UI/Contact/divs/person.html +++ b/UI/Contact/divs/person.html @@ -149,7 +149,7 @@ value="save_person" } %] [% - IF entity_id.defined AND NOT company.is_used ; + IF may_delete AND entity_id.defined AND NOT company.is_used ; INCLUDE button element_data = { text = text('Delete') id = 'person-action-delete' diff --git a/lib/LedgerSMB/Entity.pm b/lib/LedgerSMB/Entity.pm index d1e26baeb8..0279de96f2 100644 --- a/lib/LedgerSMB/Entity.pm +++ b/lib/LedgerSMB/Entity.pm @@ -95,6 +95,18 @@ sub get{ return __PACKAGE__->new(%$result); } +=item del() + +=cut + +sub del { + my ($self) = @_; + my ($result) = __PACKAGE__->call_procedure(funcname => 'entity__delete', + args => [ $self->id ]); + + return $result; +} + =back =head1 LICENSE AND COPYRIGHT diff --git a/lib/LedgerSMB/Scripts/contact.pm b/lib/LedgerSMB/Scripts/contact.pm index 9766341276..34e2bae55b 100644 --- a/lib/LedgerSMB/Scripts/contact.pm +++ b/lib/LedgerSMB/Scripts/contact.pm @@ -18,6 +18,8 @@ This module is the UI controller for the customer, vendor, etc functions; it =cut +use HTTP::Status qw( HTTP_FOUND ); + use LedgerSMB; use LedgerSMB::Entity::Company; use LedgerSMB::Entity::Person; @@ -61,6 +63,23 @@ for (@pluginmods){ =over +=item delete_entity + +=cut + +sub delete_entity { + my ($request) = @_; + my $entity = + LedgerSMB::Entity::Company->get_by_cc($request->{control_code}); + $entity ||= LedgerSMB::Entity::Person->get_by_cc($request->{control_code}); + + $entity->del; + return [ HTTP_FOUND, + [ 'Location' => 'reports.pl?__action=start_report&report_name=contact_search' ], + [ '' ] + ]; +} + =item get_by_cc Populates the company area with info on the company, pulled up through the @@ -167,6 +186,7 @@ sub _main_screen { $request->{target_div} ||= 'person_div' if defined $person; $request->{target_div} ||= 'company_div'; + my $may_delete = $request->is_allowed_role({ allowed_roles => [ 'contact_delete' ] }); my @all_managers = map { $_->{label} = "$_->{first_name} $_->{last_name}"; $_ } ($request->call_procedure( funcname => 'employee__all_managers' ),); @@ -365,7 +385,8 @@ sub _main_screen { all_managers => \@all_managers, default_country => $default_country, default_language => $default_language, - earn_id => $earn_id + earn_id => $earn_id, + may_delete => $may_delete, }); } diff --git a/sql/modules/BLACKLIST b/sql/modules/BLACKLIST index d7750c1d21..3706ba8a87 100644 --- a/sql/modules/BLACKLIST +++ b/sql/modules/BLACKLIST @@ -205,6 +205,7 @@ employee__list_managers employee__save employee__search employee_search +entity__delete entity__delete_bank_account entity__delete_contact entity__delete_location diff --git a/sql/modules/Entity.sql b/sql/modules/Entity.sql index c407878adb..c95ba1de65 100644 --- a/sql/modules/Entity.sql +++ b/sql/modules/Entity.sql @@ -111,6 +111,25 @@ COMMENT ON FUNCTION entity__get ( $$ Returns a set of (only one) entity record with the entity id.$$; +CREATE OR REPLACE FUNCTION entity__delete(in_id int) + RETURNS boolean + security definer +AS $$ +BEGIN + delete from entity where id = in_id; + return found; +END; +$$ language plpgsql; + +REVOKE EXECUTE ON FUNCTION entity__delete(in_id int) FROM public; + + +COMMENT ON FUNCTION entity__delete(in_id int) IS + $$Removes an entity and its master data. + + Removal will fail if the function 'entity__is_used()' returns 'true'. + $$; + CREATE OR REPLACE FUNCTION eca__get_entity ( in_credit_id int ) RETURNS setof entity AS $$ diff --git a/sql/modules/Roles.sql b/sql/modules/Roles.sql index 3c9961f634..d720fba239 100644 --- a/sql/modules/Roles.sql +++ b/sql/modules/Roles.sql @@ -596,6 +596,7 @@ SELECT lsmb__create_role('contact_delete', does not provide any rights. $DOC$ ); +SELECT lsmb__grant_exec('contact_delete', 'entity__delete(in_id int)'); SELECT lsmb__grant_perms('contact_delete', obj, 'DELETE') FROM unnest(ARRAY['entity'::text, 'company', 'person', 'location', 'entity_credit_account', 'eca_tax', 'entity_note', From f4f2b4d6c58597703f7164a5d6ad70b8ce891bb4 Mon Sep 17 00:00:00 2001 From: Erik Huelsmann Date: Sat, 25 Jan 2025 22:54:28 +0100 Subject: [PATCH 06/12] Exclude note and file attachments from deletion --- .../1.13/fk-actions-for-is_used-functions.sql | 25 ++++--------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/sql/changes/1.13/fk-actions-for-is_used-functions.sql b/sql/changes/1.13/fk-actions-for-is_used-functions.sql index ca58e730bd..feb6363cdd 100644 --- a/sql/changes/1.13/fk-actions-for-is_used-functions.sql +++ b/sql/changes/1.13/fk-actions-for-is_used-functions.sql @@ -13,6 +13,11 @@ deletion of the cr_coa_to_account row deletion fails due to further foreign keys being checked (and failing). + Consensus is that notes and files constitute important data that should block + deletion of the records they're attached to. This removes file_eca and + file_entity from the list of tables which need a CASCADE-ing foreign key. The + same applies to entity_note and eca_note. + */ @@ -44,18 +49,6 @@ ALTER TABLE entity_bank_account DROP CONSTRAINT entity_bank_account_entity_id_fkey, ADD FOREIGN KEY (entity_id) REFERENCES entity (id) ON DELETE CASCADE; -ALTER TABLE entity_note - DROP CONSTRAINT entity_note_entity_id_fkey, - ADD FOREIGN KEY (entity_id) REFERENCES entity (id) ON DELETE CASCADE; - -ALTER TABLE entity_note - DROP CONSTRAINT entity_note_ref_key_fkey, - ADD FOREIGN KEY (ref_key) REFERENCES entity (id) ON DELETE CASCADE; - -ALTER TABLE file_entity - DROP CONSTRAINT file_entity_ref_key_fkey, - ADD FOREIGN KEY (ref_key) REFERENCES entity (id) ON DELETE CASCADE; - -- entity type specialization ALTER TABLE company DROP CONSTRAINT company_entity_id_fkey, @@ -94,18 +87,10 @@ ALTER TABLE eca_to_location DROP CONSTRAINT eca_to_location_credit_id_fkey, ADD FOREIGN KEY (credit_id) REFERENCES entity_credit_account (id) ON DELETE CASCADE; -ALTER TABLE eca_note - DROP CONSTRAINT eca_note_ref_key_fkey, - ADD FOREIGN KEY (ref_key) REFERENCES entity_credit_account (id) ON DELETE CASCADE; - ALTER TABLE eca_tax DROP CONSTRAINT eca_tax_eca_id_fkey, ADD FOREIGN KEY (eca_id) REFERENCES entity_credit_account (id) ON DELETE CASCADE; -ALTER TABLE file_eca - DROP CONSTRAINT file_eca_ref_key_fkey, - ADD FOREIGN KEY (ref_key) REFERENCES entity_credit_account (id) ON DELETE CASCADE; - -- (price matrix data as customer) ALTER TABLE partscustomer DROP CONSTRAINT partscustomer_credit_id_fkey, From 16fc97e4657062c4541a852f368bb741e3e9514f Mon Sep 17 00:00:00 2001 From: Erik Huelsmann Date: Fri, 31 Jan 2025 20:32:01 +0100 Subject: [PATCH 07/12] Remove unused 'company__get_all_accounts' This function duplicates 'entity__list_credit'. --- sql/modules/BLACKLIST | 1 - sql/modules/Company.sql | 18 ------------------ 2 files changed, 19 deletions(-) diff --git a/sql/modules/BLACKLIST b/sql/modules/BLACKLIST index 3706ba8a87..2aaa0eff14 100644 --- a/sql/modules/BLACKLIST +++ b/sql/modules/BLACKLIST @@ -139,7 +139,6 @@ cogs__add_for_ar_line cogs__reverse_ap cogs__reverse_ar company__get -company__get_all_accounts company__get_by_cc company__next_id company__save diff --git a/sql/modules/Company.sql b/sql/modules/Company.sql index 7d909c8023..0e8a43c045 100644 --- a/sql/modules/Company.sql +++ b/sql/modules/Company.sql @@ -1434,24 +1434,6 @@ COMMENT ON FUNCTION eca__save_contact in_old_contact text, in_old_contact_class int) IS $$ Saves the contact record at the entity credit account level. Returns 1.$$; -CREATE OR REPLACE FUNCTION company__get_all_accounts ( - in_entity_id int, - in_entity_class int -) RETURNS SETOF entity_credit_account AS $body$ - - SELECT * - FROM entity_credit_account - WHERE entity_id = $1 - AND entity_class = $2; - -$body$ language SQL; - -COMMENT ON FUNCTION company__get_all_accounts ( - in_entity_id int, - in_entity_class int -) IS -$$ Returns a list of all entity credit accounts attached to that entity.$$; - -- pricematrix CREATE OR REPLACE FUNCTION eca__get_pricematrix_by_pricegroup(in_credit_id int) From e2daf133dafa8e0a589dc30f58234f35a7273623 Mon Sep 17 00:00:00 2001 From: Erik Huelsmann Date: Fri, 31 Jan 2025 20:33:02 +0100 Subject: [PATCH 08/12] Move 'entity__list_credit' to Entity.sql module --- sql/modules/Company.sql | 59 --------------------------------------- sql/modules/Entity.sql | 61 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 59 deletions(-) diff --git a/sql/modules/Company.sql b/sql/modules/Company.sql index 0e8a43c045..9cae1d8470 100644 --- a/sql/modules/Company.sql +++ b/sql/modules/Company.sql @@ -558,35 +558,6 @@ CREATE TYPE entity_credit_search_return AS ( threshold numeric ); -DROP TYPE IF EXISTS entity_credit_retrieve CASCADE; - -CREATE TYPE entity_credit_retrieve AS ( - id int, - entity_id int, - entity_class int, - discount numeric, - discount_terms int, - taxincluded bool, - creditlimit numeric, - terms int2, - meta_number text, - description text, - business_id int, - language_code text, - pricegroup_id int, - curr text, - startdate date, - enddate date, - ar_ap_account_id int, - cash_account_id int, - discount_account_id int, - threshold numeric, - control_code text, - credit_id int, - pay_to_name text, - taxform_id int -); - COMMENT ON TYPE entity_credit_search_return IS $$ This may change in 1.4 and should not be relied upon too much $$; @@ -606,36 +577,6 @@ $$ Returns an entity credit id, based on entity_id, entity_class, and meta_number. This is the preferred way to locate an account if all three of these are known$$; -CREATE OR REPLACE FUNCTION entity__list_credit -(in_entity_id int, in_entity_class int) -RETURNS SETOF entity_credit_retrieve AS -$$ -BEGIN -RETURN QUERY EXECUTE $sql$ - SELECT ec.id, e.id, ec.entity_class, ec.discount, - ec.discount_terms, - ec.taxincluded, ec.creditlimit, ec.terms, - ec.meta_number::text, ec.description, ec.business_id, - ec.language_code::text, - ec.pricegroup_id, ec.curr::text, ec.startdate, - ec.enddate, ec.ar_ap_account_id, ec.cash_account_id, - ec.discount_account_id, - ec.threshold, e.control_code, ec.id, ec.pay_to_name, - ec.taxform_id - FROM entity e - JOIN entity_credit_account ec ON (e.id = ec.entity_id) - WHERE e.id = $1 - AND (ec.entity_class = $2 - or $2 is null) -$sql$ -USING in_entity_id, in_entity_class; -END -$$ LANGUAGE PLPGSQL; - -COMMENT ON FUNCTION entity__list_credit (in_entity_id int, in_entity_class int) -IS $$ Returns a list of entity credit account entries for the entity and of the -entity class.$$; - CREATE OR REPLACE FUNCTION company__get (in_entity_id int) RETURNS company_entity AS $$ diff --git a/sql/modules/Entity.sql b/sql/modules/Entity.sql index c95ba1de65..0ff3ebdda0 100644 --- a/sql/modules/Entity.sql +++ b/sql/modules/Entity.sql @@ -99,6 +99,67 @@ $$ LANGUAGE PLPGSQL; COMMENT ON FUNCTION entity__list_classes () IS $$ Returns a list of entity classes, ordered by assigned ids$$; + +DROP TYPE IF EXISTS entity_credit_retrieve CASCADE; + +CREATE TYPE entity_credit_retrieve AS ( + id int, + entity_id int, + entity_class int, + discount numeric, + discount_terms int, + taxincluded bool, + creditlimit numeric, + terms int2, + meta_number text, + description text, + business_id int, + language_code text, + pricegroup_id int, + curr text, + startdate date, + enddate date, + ar_ap_account_id int, + cash_account_id int, + discount_account_id int, + threshold numeric, + control_code text, + credit_id int, + pay_to_name text, + taxform_id int, + is_used boolean +); + +CREATE OR REPLACE FUNCTION entity__list_credit +(in_entity_id int, in_entity_class int) +RETURNS SETOF entity_credit_retrieve AS +$$ +BEGIN +RETURN QUERY EXECUTE $sql$ + SELECT ec.id, e.id, ec.entity_class, ec.discount, + ec.discount_terms, + ec.taxincluded, ec.creditlimit, ec.terms, + ec.meta_number::text, ec.description, ec.business_id, + ec.language_code::text, + ec.pricegroup_id, ec.curr::text, ec.startdate, + ec.enddate, ec.ar_ap_account_id, ec.cash_account_id, + ec.discount_account_id, + ec.threshold, e.control_code, ec.id, ec.pay_to_name, + ec.taxform_id, eca__is_used(ec.id) + FROM entity e + JOIN entity_credit_account ec ON (e.id = ec.entity_id) + WHERE e.id = $1 + AND (ec.entity_class = $2 + or $2 is null) +$sql$ +USING in_entity_id, in_entity_class; +END +$$ LANGUAGE PLPGSQL; + +COMMENT ON FUNCTION entity__list_credit (in_entity_id int, in_entity_class int) +IS $$ Returns a list of entity credit account entries for the entity and of the +entity class.$$; + CREATE OR REPLACE FUNCTION entity__get ( in_entity_id int ) RETURNS setof entity AS $$ From d0a454dd497266f937755f30bd871db8b6d7c69e Mon Sep 17 00:00:00 2001 From: Erik Huelsmann Date: Fri, 31 Jan 2025 21:14:37 +0100 Subject: [PATCH 09/12] Add 'eca__delete(int)' for deletion of unused credit accounts --- sql/modules/BLACKLIST | 1 + sql/modules/Entity.sql | 19 +++++++++++++++++++ sql/modules/Roles.sql | 1 + 3 files changed, 21 insertions(+) diff --git a/sql/modules/BLACKLIST b/sql/modules/BLACKLIST index 2aaa0eff14..6d092bbd38 100644 --- a/sql/modules/BLACKLIST +++ b/sql/modules/BLACKLIST @@ -175,6 +175,7 @@ draft__delete_lines draft__search draft_approve draft_delete +eca__delete eca__delete_contact eca__delete_location eca__delete_pricematrix diff --git a/sql/modules/Entity.sql b/sql/modules/Entity.sql index 0ff3ebdda0..9fdf888e06 100644 --- a/sql/modules/Entity.sql +++ b/sql/modules/Entity.sql @@ -191,6 +191,25 @@ COMMENT ON FUNCTION entity__delete(in_id int) IS Removal will fail if the function 'entity__is_used()' returns 'true'. $$; +CREATE OR REPLACE FUNCTION eca__delete(in_id int) + RETURNS boolean + security definer +AS $$ +BEGIN + delete from entity_credit_account where id = in_id; + return found; +END; +$$ language plpgsql; + +REVOKE EXECUTE ON FUNCTION eca__delete(in_id int) FROM PUBLIC; + +COMMENT ON FUNCTION eca__delete(in_id int) IS + $$Removes an entity credit account and its master data. + + Removal will fail if the function 'eca__is_used()' returns 'true'. + $$; + + CREATE OR REPLACE FUNCTION eca__get_entity ( in_credit_id int ) RETURNS setof entity AS $$ diff --git a/sql/modules/Roles.sql b/sql/modules/Roles.sql index d720fba239..59e0ba9dc2 100644 --- a/sql/modules/Roles.sql +++ b/sql/modules/Roles.sql @@ -597,6 +597,7 @@ SELECT lsmb__create_role('contact_delete', $DOC$ ); SELECT lsmb__grant_exec('contact_delete', 'entity__delete(in_id int)'); +SELECT lsmb__grant_exec('contact_delete', 'eca__delete(in_id int)'); SELECT lsmb__grant_perms('contact_delete', obj, 'DELETE') FROM unnest(ARRAY['entity'::text, 'company', 'person', 'location', 'entity_credit_account', 'eca_tax', 'entity_note', From 8059b918bbe91141b79d1359a4d2dc09aa939bc2 Mon Sep 17 00:00:00 2001 From: Erik Huelsmann Date: Fri, 31 Jan 2025 21:15:28 +0100 Subject: [PATCH 10/12] Add back 'Save' button lost when adding the 'Delete' button --- UI/Contact/divs/company.html | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/UI/Contact/divs/company.html b/UI/Contact/divs/company.html index f5b1b6dc4b..396a0fec5c 100644 --- a/UI/Contact/divs/company.html +++ b/UI/Contact/divs/company.html @@ -133,6 +133,14 @@ value = 'get_by_cc' class = 'submit' } %] + [% INCLUDE button element_data = { + text = text('Save') + id = 'company-action-save' + class="submit" + type="submit" + name="__action" + value="save_company" + }; IF may_delete AND entity_id.defined AND NOT company.is_used ; INCLUDE button element_data = { text = text('Delete') From 30a1efee76018fbdf56ffa6146a7af80e54a1707 Mon Sep 17 00:00:00 2001 From: Erik Huelsmann Date: Fri, 31 Jan 2025 21:18:57 +0100 Subject: [PATCH 11/12] Implement deletion of credit accounts from the UI --- UI/Contact/divs/credit.html | 14 ++++++++++---- lib/LedgerSMB/Entity/Credit_Account.pm | 21 +++++++++++++++++++++ lib/LedgerSMB/Scripts/contact.pm | 17 +++++++++++++++++ 3 files changed, 48 insertions(+), 4 deletions(-) diff --git a/UI/Contact/divs/credit.html b/UI/Contact/divs/credit.html index 49371fdeb4..7271537740 100644 --- a/UI/Contact/divs/credit.html +++ b/UI/Contact/divs/credit.html @@ -15,8 +15,12 @@ cl.account_class_label = text('Customer'); END; cl.meta_number_href_suffix = 'account_class=' _ cl.entity_class _ - '&entity_id=' _ cl.entity_id _ '&meta_number=' _ cl.meta_number _ - '&target_div=credit_div'; + '&entity_id=' _ cl.entity_id _ '&meta_number=' _ cl.meta_number; + IF NOT cl.is_used ; + cl.delete_href_suffix = 'entity_id=' _ cl.entity_id _ + '&credit_id=' _ cl.id; + cl.delete = '[ ' _ text('Delete') _ ' ]'; + END; END; PROCESS dynatable @@ -24,16 +28,18 @@ columns = [ { col_id = 'account_class_label', type = 'text', name = text('Type') } { col_id = 'meta_number', type = 'href', name = text("Number"), - href_base = "$request.script?__action=get&" } + href_base = "$request.script?__action=get&target_div=credit_div&" } { col_id = 'description', type = 'text', name = text("Description") } { col_id = 'creditlimit', type = 'text', name = text('Credit Limit') } { col_id = 'startdate', type = 'text', name = text('Start Date') } { col_id = 'enddate', type = 'text', name = text('End Date') } + { col_id = 'delete', type = 'href', + href_base = "$request.script?__action=delete_credit&target_div=credit_div&" } ], #' tbody = {rows = credit_list}; %]
-[% PROCESS input element_data = { + [% PROCESS input element_data = { type = "hidden" name = "form_id" value = form_id diff --git a/lib/LedgerSMB/Entity/Credit_Account.pm b/lib/LedgerSMB/Entity/Credit_Account.pm index 49aa10b9df..003c992fae 100644 --- a/lib/LedgerSMB/Entity/Credit_Account.pm +++ b/lib/LedgerSMB/Entity/Credit_Account.pm @@ -270,6 +270,15 @@ This is the tax reporting form associated with the account. has 'taxform_id' => (is => 'rw', isa => 'Maybe[Int]', required => 0); +=item is_used + +Boolean indicating whether the credit account is used (and therefor not +deletable). + +=cut + +has 'is_used' => (is => 'ro', isa => 'Bool', required => 0); + =back =head1 METHODS @@ -364,6 +373,18 @@ sub get_current_debt { return $self->current_debt; } +=item del() + +Removes the entity credit account. + +=cut + +sub del { + my ($self) = @_; + + $self->call_dbmethod(funcname => 'eca__delete'); +} + =item save() Saves the entity credit account. This also sets db defaults if not set. diff --git a/lib/LedgerSMB/Scripts/contact.pm b/lib/LedgerSMB/Scripts/contact.pm index 34e2bae55b..c293a591ec 100644 --- a/lib/LedgerSMB/Scripts/contact.pm +++ b/lib/LedgerSMB/Scripts/contact.pm @@ -621,6 +621,23 @@ sub save_person { return _main_screen($request, undef, $person); } +=item delete_credit($request) + +Deletes the credit account indicated by C<< $request->{credit_id} >>, if the user +has sufficient access rights. + +=cut + +sub delete_credit { + my ($request) = @_; + + my $credit = LedgerSMB::Entity::Credit_Account->get_by_id( $request->{credit_id} ); + if ($credit) { + $credit->del; + } + return get($request); +} + =item save_credit($request) This inserts or updates a credit account of the sort listed here. From 503396e1470bb30abedf4fb56f65cba55fce40cb Mon Sep 17 00:00:00 2001 From: Erik Huelsmann Date: Fri, 31 Jan 2025 21:23:07 +0100 Subject: [PATCH 12/12] Remove test for removed function --- xt/42-company.pg | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/xt/42-company.pg b/xt/42-company.pg index bcefabd3f5..438785ac68 100644 --- a/xt/42-company.pg +++ b/xt/42-company.pg @@ -5,7 +5,7 @@ BEGIN; -- Plan the tests. - SELECT plan(58); + SELECT plan(57); -- Add data @@ -19,7 +19,6 @@ BEGIN; -- Validate required functions SELECT has_function('account__all_headings','{}'::text[]); - SELECT has_function('company__get_all_accounts',ARRAY['integer','integer']); SELECT has_function('company_get_billing_info',ARRAY['integer']); SELECT has_function('company__get_by_cc',ARRAY['text']); SELECT has_function('company__get',ARRAY['integer']);