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 duplicated endorsements #11853

Merged
merged 7 commits into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions RELEASE_NOTES.md
Expand Up @@ -285,6 +285,16 @@ bundle exec rails decidim:robots:replace

You can see more details about this change on PR [\#11693](https://github.com/decidim/decidim/pull/11693)

### 3.12. Deduplicating endorsements

We have identified a case when the same user can endorse the same resource multiple times. This is a bug that we have fixed in this release, but we need to clean up the existing duplicated endorsements. We have added a new task that helps you clean the duplicated endorsements.

```bash
bundle exec rails decidim:upgrade:fix_duplicate_endorsements
```

You can see more details about this change on PR [\#11853](https://github.com/decidim/decidim/pull/11853)

## 4. Scheduled tasks

Implementers need to configure these changes it in your scheduler task system in the production server. We give the examples
Expand Down
2 changes: 2 additions & 0 deletions decidim-core/app/commands/decidim/endorse_resource.rb
Expand Up @@ -31,6 +31,8 @@ def call
else
broadcast(:invalid)
end
rescue ActiveRecord::RecordNotUnique
broadcast(:invalid)
end

private
Expand Down
2 changes: 1 addition & 1 deletion decidim-core/app/commands/decidim/unendorse_resource.rb
Expand Up @@ -31,7 +31,7 @@ def destroy_resource_endorsement
query = if @current_group.present?
@resource.endorsements.where(decidim_user_group_id: @current_group&.id)
else
@resource.endorsements.where(author: @current_user, decidim_user_group_id: nil)
@resource.endorsements.where(author: @current_user, decidim_user_group_id: 0)
end
query.destroy_all
end
Expand Down
@@ -0,0 +1,11 @@
# frozen_string_literal: true

class ChangeDefaultValueForDecidimEndorsements < ActiveRecord::Migration[6.1]
def up
change_column_default :decidim_endorsements, :decidim_user_group_id, 0
end

def down
change_column_default :decidim_endorsements, :decidim_user_group_id, nil
end
end
2 changes: 1 addition & 1 deletion decidim-core/lib/decidim/endorsable.rb
Expand Up @@ -21,7 +21,7 @@ def endorsed_by?(user, user_group = nil)
if user_group
endorsements.where(user_group:).any?
else
endorsements.where(author: user, user_group: nil).any?
endorsements.where(author: user, user_group: 0).any?
end
end
end
Expand Down
@@ -0,0 +1,53 @@
# frozen_string_literal: true

namespace :decidim do
namespace :upgrade do
desc "Modify nicknames with random numbers when exists similar ones case insensitively"
alecslupu marked this conversation as resolved.
Show resolved Hide resolved
alecslupu marked this conversation as resolved.
Show resolved Hide resolved
task fix_duplicate_endorsements: :environment do
logger = Logger.new($stdout)
logger.info("Removing duplicate endorsements...")
has_count = 0

columns = [:resource_type, :resource_id, :decidim_author_type, :decidim_author_id, :decidim_user_group_id]

get_duplicates(columns).each do |issue|
while row_count(issue) > 1
find_next(issue)&.destroy
has_count += 1
logger.info("Removed duplicate endorsement for #{issue.resource_type} #{issue.resource_id}")
end
end

logger.info("Patch remaining endorsements.")
Decidim::Endorsement.where(decidim_user_group_id: nil).update(decidim_user_group_id: 0)
logger.info("Process terminated, #{has_count} endorsements have been removed.")
logger.info("Done")
end

private

def get_duplicates(*columns)
Decidim::Endorsement.select("#{columns.join(",")}, COUNT(*)").group(columns).having("COUNT(*) > 1")
end

def row_count(issue)
Decidim::Endorsement.where(
resource_type: issue.resource_type,
resource_id: issue.resource_id,
decidim_author_type: issue.decidim_author_type,
decidim_author_id: issue.decidim_author_id,
decidim_user_group_id: issue.decidim_user_group_id
).count
end

def find_next(issue)
Decidim::Endorsement.find_by(
resource_type: issue.resource_type,
resource_id: issue.resource_id,
decidim_author_type: issue.decidim_author_type,
decidim_author_id: issue.decidim_author_id,
decidim_user_group_id: issue.decidim_user_group_id
)
end
end
end
4 changes: 2 additions & 2 deletions decidim-core/spec/models/decidim/endorsement_spec.rb
Expand Up @@ -86,8 +86,8 @@ module Decidim

it "sorts user_grup endorsements first and then by created_at" do
alecslupu marked this conversation as resolved.
Show resolved Hide resolved
expected_sorting = [
endorsement.id, other_endorsement2.id,
other_endorsement1.id
other_endorsement1.id, endorsement.id,
other_endorsement2.id
]
expect(resource.endorsements.for_listing.pluck(:id)).to eq(expected_sorting)
end
Expand Down