Merge pull request #16929 from eduardoj/refactoring/group_find_by_title

Don't override `find_by_title!` in groups

src/api/app/controllers/group_controller.rb

@@ -9,6 +9,8 @@ class GroupController < ApplicationController
   # raise an exception if authorize has not yet been called.
   after_action :verify_authorized, except: %i[index show]
 
+  rescue_from ActiveRecord::RecordNotFound, with: :record_not_found
+
   rescue_from Pundit::NotAuthorizedError do |exception|
     pundit_action = case exception.query.to_s
                     when 'index?' then 'list'
@@ -83,4 +85,10 @@ def command
 
     render_ok
   end
+
+  private
+
+  def record_not_found
+    render_error status: 404, message: "Couldn't find Group '#{params[:title]}'"
+  end
 end

src/api/app/controllers/webui/groups_controller.rb

@@ -9,8 +9,6 @@ def index
 
   def show
     @group = Group.includes(:users).find_by_title(params[:title])
-
-    # Group.find_by_title! is self implemented and would raise an 500 error
     return if @group
 
     flash[:error] = "Group '#{params[:title]}' does not exist"

src/api/app/models/group.rb

@@ -40,12 +40,6 @@ class Group < ApplicationRecord
 
   alias_attribute :name, :title
 
-  def self.find_by_title!(title)
-    find_by!(title: title)
-  rescue ActiveRecord::RecordNotFound => e
-    raise e, "Couldn't find Group '#{title}'", e.backtrace
-  end
-
   def update_from_xml(xmlhash, user_session_login:)
     with_lock do
       self.email = xmlhash.value('email')

src/api/spec/models/bs_request/find_for/group_spec.rb

@@ -21,7 +21,7 @@
     context 'with a not existing group' do
       subject { klass.new(group: 'not-existent') }
 
-      it { expect { subject.all }.to raise_error(ActiveRecord::RecordNotFound, "Couldn't find Group 'not-existent'") }
+      it { expect { subject.all }.to raise_error(ActiveRecord::RecordNotFound) }
     end
 
     context 'with a group maintainer relationship' do

src/api/spec/shared/examples/features/bootstrap_user_tab.rb

@@ -166,7 +166,7 @@
         click_button('Accept')
       end
 
-      expect(page).to have_text("Couldn't find Group 'unknown group'")
+      expect(page).to have_text("Couldn't find Group")
     end
 
     it 'Add an existing group' do