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

#4739: Fix product drive delete (updated) #4918

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
39c7550
add new logic for product_drive deletion
jorgecoutinhobr Nov 1, 2024
3aec94e
add new tests for Product Drive system spec
jorgecoutinhobr Nov 1, 2024
cb17ab1
Implement new role verification and validation for product drive dele…
jorgecoutinhobr Nov 4, 2024
f7b416a
Remove product drive deletion tests from system spec
jorgecoutinhobr Nov 4, 2024
65d9b7e
Merge branch 'main' of https://github.com/MichaScant/human-essentials…
MichaScant Dec 18, 2024
83976b7
fixed proposed changes
MichaScant Jan 3, 2025
ea47123
Merge branch 'main' of https://github.com/MichaScant/human-essentials…
MichaScant Jan 6, 2025
3204abe
proposed changes
MichaScant Jan 10, 2025
677de12
fixed ruby lint errors
MichaScant Jan 10, 2025
b0ac10f
fixed error in rspec-system(6,3)
MichaScant Jan 10, 2025
045d8be
replaced ProductDrivePolicy with the can destroy in product destroy s…
MichaScant Jan 10, 2025
018c9f4
changed can_destroy to class method so it is able to be called
MichaScant Jan 10, 2025
1b8c05f
fixed rubocop errors and arguments in show.html.erb
MichaScant Jan 13, 2025
4d822f3
attempted to fixes, made can_destroy class method ot make it a class …
MichaScant Jan 13, 2025
542a1e7
removed redundant operation as product_drive is already accessible
MichaScant Jan 13, 2025
cbb18ec
no longer destroying in controller
MichaScant Jan 13, 2025
4bb1407
Merge branch 'main' of https://github.com/MichaScant/human-essentials…
MichaScant Jan 13, 2025
e8edee3
updated code based on requested changes
MichaScant Jan 16, 2025
21cf464
throwing errors in destroy service at times now as test expects it
MichaScant Jan 16, 2025
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
11 changes: 7 additions & 4 deletions app/controllers/product_drives_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,13 @@ def update
end

def destroy
current_organization.product_drives.find(params[:id]).destroy
respond_to do |format|
format.html { redirect_to product_drives_url, notice: 'Product drive was successfully destroyed.' }
format.json { head :no_content }
result = ProductDriveDestroyService.call(@product_drive, current_user, current_organization)

if result.success?
redirect_to product_drives_url, notice: result.value
else
flash[:error] = result.error
redirect_back(fallback_location: product_drives_url)
end
end

Expand Down
29 changes: 29 additions & 0 deletions app/services/product_drive_destroy_service.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
class ProductDriveDestroyService
class << self
def call(product_drive, user, organization)
return Result.new(error: "You are not allowed to perform this action.") unless verify_role(user, organization)

unless can_destroy?(product_drive, user)
product_drive.errors.add(:base, "Cannot delete product drive with donations.")
raise ActiveRecord::RecordInvalid.new(product_drive)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we sometimes raising an error and sometimes setting the error to the Result object?

end

if product_drive.destroy
Result.new(value: "Product drive was successfully destroyed.")
else
raise ActiveRecord::RecordNotDestroyed.new("Failed to destroy product drive", product_drive)
end
end

def can_destroy?(product_drive, user)
return false unless user.has_role?(Role::ORG_ADMIN, product_drive.organization)
product_drive.donations.empty?
end

private

def verify_role(user, organization)
user.has_role?(Role::ORG_ADMIN, organization)
end
end
end
2 changes: 1 addition & 1 deletion app/views/product_drives/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
</p>
<div class="card-footer clearfix">
<%= edit_button_to edit_product_drive_path(@product_drive), { text: "Make a correction", size: "md" } %>
<%= delete_button_to product_drive_path(@product_drive), { confirm: "Are you sure you want to permanently remove this product drive?", size: "md" } if current_user.has_role?(Role::ORG_ADMIN, current_organization) %>
<%= delete_button_to product_drive_path(@product_drive), { confirm: "Are you sure you want to permanently remove this product drive?", size: "md", enabled: ProductDriveDestroyService.can_destroy?(@product_drive, current_user)} %>
</div>
<!-- /.card-footer-->
</div>
Expand Down
Loading