Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: refactor GroupController to remove code duplication #346

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 17 additions & 13 deletions app/controllers/groups_controller.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
class GroupsController < ApplicationController
before_action :assure_signed_in
before_action :set_user_group
before_action :set_group, only: %i[ edit update destroy ]
before_action :assure_admin, only: %i[ edit update destroy ]
before_action :assure_signed_in, :set_user_group
before_action :set_group, :assure_admin, only: %i[ edit update destroy ]

# GET /groups or /groups.json
def index
Expand All @@ -26,8 +24,8 @@ def create
respond_to do |format|
if @group.save
Membership.create(user: current_user, group: @group, role: :admin)
format.html { redirect_to groups_url, notice: t(:group_new) }
format.json { render :show, status: :created, location: @group }
respond_with_notice_and_status(format, redirect: edit_group_url(@group), notice: t(:group_new),
status: :created)
else
unprocessable_response(format, redirect: :index, entity: @group)
end
Expand All @@ -38,8 +36,7 @@ def create
def update
respond_to do |format|
if @group.update(group_params)
format.html { redirect_to edit_group_url(@group), notice: t(:group_update) }
format.json { render :show, status: :ok, location: @group }
respond_with_notice_and_status(format, redirect: edit_group_url(@group), notice: t(:group_update), status: :ok)
else
unprocessable_response(format, redirect: :edit, entity: @group)
end
Expand All @@ -52,17 +49,15 @@ def destroy
@group.destroy

respond_to do |format|
format.html { redirect_to groups_url, notice: t(:group_destroy) }
format.json { head :no_content }
respond_with_notice(format, redirect: groups_url, notice: t(:group_destroy))
end
end

# POST /groups/1/leave or /groups/1/leave.json
def leave
respond_to do |format|
if current_user.memberships.destroy_by(group_id: params[:group_id])
format.html { redirect_to groups_url, notice: t(:group_update) }
format.json { head :no_content }
respond_with_notice(format, redirect: groups_url, notice: t(:group_update))
else
unprocessable_response(format, redirect: :edit, entity: @group)
end
Expand Down Expand Up @@ -93,7 +88,6 @@ def set_user_group
@groups = current_user.groups
end

# Use callbacks to share common setup or constraints between actions.
def set_group
@group = Group.find(params[:id])
end
Expand All @@ -103,6 +97,16 @@ def unprocessable_response(format, redirect:, entity:)
format.json { render json: entity.errors, status: :unprocessable_entity }
end

def respond_with_notice(format, redirect:, notice:)
format.html { redirect_to redirect, notice: notice }
format.json { head :no_content }
end

def respond_with_notice_and_status(format, redirect:, notice:, status:)
format.html { redirect_to redirect, notice: notice }
format.json { render :show, status: status, location: @group }
end

# Only allow a list of trusted parameters through.
def group_params
params.require(:group).permit(:name)
Expand Down
4 changes: 4 additions & 0 deletions spec/requests/groups_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,12 @@
end.to change(Group, :count).by(1)
end

# currently to all groups, because show is not available
it "redirects to the created group" do
post groups_url, params: { group: valid_attributes }
#test_group = Group.find {|g| g.name = valid_attributes[:name] }


expect(response).to redirect_to(groups_url)
end
end
Expand Down