Merge pull request metabrainz#1931 from reosarevok/MBS-10076

MBS-10076 / MBS-11403: Friendlier error when deleting attribute that has children

Author: reosarevok

lib/MusicBrainz/Server/Controller/Admin/Attributes.pm

@@ -1,6 +1,8 @@
 package MusicBrainz::Server::Controller::Admin::Attributes;
 use Moose;
 
+use MusicBrainz::Server::Translation qw( l ln );
+
 no if $] >= 5.018, warnings => "experimental::smartmatch";
 
 BEGIN { extends 'MusicBrainz::Server::Controller' };
@@ -115,10 +117,28 @@ sub delete : Chained('attribute_base') Args(1) RequireAuth(account_admin) Secure
     $c->stash->{attribute} = $attr;
 
     if ($c->model($model)->in_use($id)) {
-        $c->stash->{template} = 'admin/attributes/in_use.tt';
+        my $error_message = l('You cannot remove the attribute "{name}" because it is still in use.', { name => $attr->name });
+
+        $c->stash(
+            current_view => 'Node',
+            component_path => 'admin/attributes/CannotRemoveAttribute',
+            component_props => {message => $error_message}
+        );
+        
         $c->detach;
     }
 
+    if ($c->model($model)->has_children($id)) {
+        my $error_message = l('You cannot remove the attribute “{name}” because it is the parent of other attributes.', { name => $attr->name });
+
+        $c->stash(
+            current_view => 'Node',
+            component_path => 'admin/attributes/CannotRemoveAttribute',
+            component_props => {message => $error_message}
+        );
+        
+        $c->detach;
+    }
     if ($c->form_posted_and_valid($form)) {
         $c->model('MB')->with_transaction(sub {
             $c->model($model)->delete($id);

lib/MusicBrainz/Server/Data/Role/Attribute.pm

@@ -19,6 +19,13 @@ sub _column_mapping {
     };
 }
 
+sub has_children {
+    my ($self, $id) = @_;
+    return $self->sql->select_single_value(
+        'SELECT 1 FROM ' . $self->_table . ' WHERE parent = ? LIMIT 1',
+        $id);
+}
+
 no Moose;
 1;
 

root/admin/attributes/CannotRemoveAttribute.js

@@ -0,0 +1,31 @@
+/*
+ * @flow strict-local
+ * Copyright (C) 2021 MetaBrainz Foundation
+ *
+ * This file is part of MusicBrainz, the open internet music database,
+ * and is licensed under the GPL version 2, or (at your option) any
+ * later version: http://www.gnu.org/licenses/gpl-2.0.txt
+ */
+
+import * as React from 'react';
+
+import Layout from '../../layout';
+
+type Props = {
+  +$c: CatalystContextT,
+  +message: string,
+};
+
+const CannotRemoveAttribute = ({
+  $c,
+  message,
+}: Props): React.Element<typeof Layout> => (
+  <Layout $c={$c} fullWidth title={l('Cannot Remove Attribute')}>
+    <h1>{l('Cannot Remove Attribute')}</h1>
+    <p>
+      {message}
+    </p>
+  </Layout>
+);
+
+export default CannotRemoveAttribute;

root/admin/attributes/in_use.tt

@@ -39,6 +39,7 @@ module.exports = {
   'admin/EmailSearch': require('../admin/EmailSearch'),
   'admin/IpLookup': require('../admin/IpLookup'),
   'admin/attributes/Attribute': require('../admin/attributes/Attribute'),
+  'admin/attributes/CannotRemoveAttribute': require('../admin/attributes/CannotRemoveAttribute'),
   'admin/attributes/Index': require('../admin/attributes/Index'),
   'admin/attributes/Language': require('../admin/attributes/Language'),
   'admin/attributes/Script': require('../admin/attributes/Script'),