Skip to content

Fix 'download your data' when there are comments on budgets#11531

Merged
andreslucena merged 1 commit intodevelopfrom
fix/budgets-polymorphic_resource_url
Aug 28, 2023
Merged

Fix 'download your data' when there are comments on budgets#11531
andreslucena merged 1 commit intodevelopfrom
fix/budgets-polymorphic_resource_url

Conversation

@alecslupu
Copy link
Copy Markdown
Contributor

@alecslupu alecslupu commented Aug 27, 2023 •
edited by andreslucena
Loading

🎩 What? Why?

While reviewing #11527 i have noticed that the export class is actually raise an error.

2023-08-27T08:46:56.268Z pid=1088300 tid=omi0 WARN: ArgumentError: wrong number of arguments (given 0, expected 1)
2023-08-27T08:46:56.268Z pid=1088300 tid=omi0 WARN: /home/alecslupu/Sites/decidim/redesign/decidim-budgets/app/models/decidim/budgets/project.rb:73:in `polymorphic_resource_url'
/home/alecslupu/Sites/decidim/redesign/decidim-comments/lib/decidim/comments/comment_serializer.rb:36:in `root_commentable_url'
/home/alecslupu/Sites/decidim/redesign/decidim-comments/lib/decidim/comments/comment_serializer.rb:28:in `serialize'
/home/alecslupu/Sites/decidim/redesign/decidim-core/app/serializers/decidim/exporters/serializer.rb:24:in `run'

📌 Related Issues

Link your PR to an issue

Testing

  1. Login with admin account
  2. From user profile, click request your data
  3. Process queues using sidekiq
  4. See error in sidekiq
  5. Apply patch, retry job
  6. See there is no error in sidekiq
  7. Export archive is being delivered in letter opener

♥️ Thank you!

@alecslupu alecslupu force-pushed the fix/budgets-polymorphic_resource_url branch from d10b0c8 to 4403df9 Compare August 27, 2023 09:42
@alecslupu alecslupu requested a review from a team August 27, 2023 12:41
@alecslupu alecslupu added this to the 0.28.0 milestone Aug 27, 2023
@alecslupu alecslupu marked this pull request as ready for review August 27, 2023 12:41
@alecslupu alecslupu added the type: fix PRs that implement a fix for a bug label Aug 27, 2023
@andreslucena andreslucena changed the title Fix budgets polymorphic_resource_url Fix comment export for budgets component Aug 28, 2023
@andreslucena
Copy link
Copy Markdown
Member

andreslucena commented Aug 28, 2023

I've changed the title to make it more clear and removed the unused PR template section (Screenshots)

andreslucena
andreslucena approved these changes Aug 28, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

Tried it out locally with the instructions provided and I've also checked that the spec fails without this change:

wget https://raw.githubusercontent.com/decidim/decidim/4403df9f9885e78f7ffb1b71200b6784c9d714af/decidim-core/lib/decidim/core/test/shared_examples/comments_examples.rb -O decidim-core/lib/decidim/core/test/shared_examples/comments_examples.rb 
bin/rspec decidim-budgets/spec/system/comments_spec.rb -e "export_serializer"

Everything works correctly, great job @alecslupu!! 👏🏽 👏🏽

@andreslucena
Copy link
Copy Markdown
Member

andreslucena commented Aug 28, 2023

I will not merge until you check the title @alecslupu, maybe there's a better title still, as it isn't 100% of the budget components but of the "download your data" feature. Maybe we should change it to "Fix 'download your data' when there are comments on budgets"? Or is that too much?

@alecslupu alecslupu changed the title Fix comment export for budgets component Fix 'download your data' when there are comments on budgets Aug 28, 2023
@alecslupu
Copy link
Copy Markdown
Contributor Author

alecslupu commented Aug 28, 2023

@andreslucena , changed the title to: "Fix 'download your data' when there are comments on budgets"

@andreslucena andreslucena merged commit 8178f2e into develop Aug 28, 2023
@andreslucena andreslucena deleted the fix/budgets-polymorphic_resource_url branch August 28, 2023 08:37
entantoencuanto added a commit that referenced this pull request Aug 30, 2023
@entantoencuanto
Merge branch 'develop' into feature/redesign-admin
9524169 
* develop:
  Add recognition to BrowserStack in the README (#11546)
  Remove unused view hook for `:upcoming_meeting_for_card` (#11543)
  Remove unused dependency: `wicked` (#11150)
  Clean-up initiatives signature URLs and methods (#11545)
  Refactor initiative signing wizard (#10731)
  Fix Permissions screen on budgets throw errors (#11532)
  Redesign: read more literal (#11516)
  Fix 'download your data' when there are comments on budgets (#11531)
This was referenced Nov 2, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andreslucena andreslucena andreslucena approved these changes

Assignees

No one assigned

Labels

module: budgets module: comments type: fix PRs that implement a fix for a bug

Projects

No open projects
Maintainers' backlog
Archived in project

Milestone

0.28.0

Development

Successfully merging this pull request may close these issues.

2 participants

@alecslupu @andreslucena