Skip to content

Add initiatives export#6070

Merged
jesusdb merged 6 commits intodecidim:developfrom
aspgems:feature/export_initiatives
May 26, 2020
Merged

Add initiatives export#6070
jesusdb merged 6 commits intodecidim:developfrom
aspgems:feature/export_initiatives

Conversation

@ace
Copy link
Copy Markdown
Contributor

@ace ace commented May 5, 2020

🎩 What? Why?

Allow admins to export initiatives.

📌 Related Issues

📋 Subtasks

  • Add CHANGELOG entry
  • Add tests

📷 Screenshots (optional)

Drowpdown, links and message

@ace ace mentioned this pull request May 5, 2020
Closed
15 tasks
jesusdb
jesusdb suggested changes May 22, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@jesusdb jesusdb left a comment

Choose a reason for hiding this comment

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

Good job! Just left one suggestion

decidim-initiatives/app/jobs/decidim/initiatives/export_initiatives_job.rb
def perform(user, format)
export_data = Decidim::Exporters.find_exporter(format).new(collection, serializer).export

ExportMailer.export(user, "initiatives", export_data).deliver_now
Copy link
Copy Markdown
Contributor

@jesusdb jesusdb May 22, 2020

Choose a reason for hiding this comment

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

Suggested change
ExportMailer.export(user, "initiatives", export_data).deliver_now
ExportMailer.export(user, "initiatives", export_data).deliver_later

It would be good to use deliver_later as it's being used now to send emails

@jesusdb
Copy link
Copy Markdown
Contributor

jesusdb commented May 22, 2020

Please, check the conflicting file 😄

@ace ace force-pushed the feature/export_initiatives branch from 6ceccc2 to 68b1097 Compare May 22, 2020 12:02
@ace
Copy link
Copy Markdown
Contributor Author

ace commented May 22, 2020

Rebased develop to remove the conflicts and changed the job to deliver_later

@ace
Copy link
Copy Markdown
Contributor Author

ace commented May 22, 2020

Rebasing again to solve the new conflict.
@jesusdb I was too quick changing that deliver_now. I think this needs some extra thought/work.
As the failing spec detects, the data being passed to the mailer is invalid, we should be doing some serialization (passing .to_json or similar) and then change the mailer method to accept that. But the mailer is used by other exports (all using deliver_now at this moment).
I think it would be better to handle that change on its own PR for all the exports at once. What do you think?

@microstudi
Copy link
Copy Markdown
Contributor

microstudi commented May 22, 2020

@ace is this inside a method that it is called already in a perform_later operation? if that is the case, then you can safely leave the deliver_now right?

@ace
Copy link
Copy Markdown
Contributor Author

ace commented May 25, 2020

Hi @microstudi Yes, the export is already using a perform_later. Reverting the change (and rebasing again).

@ace ace force-pushed the feature/export_initiatives branch from 68b1097 to ba68853 Compare May 25, 2020 09:28
@jesusdb
Copy link
Copy Markdown
Contributor

jesusdb commented May 25, 2020 •
edited
Loading

Hi @ace, yes you're right, it should have its own PR. Sorry about that.

@microstudi even if it has been made by a perform_later operation, I saw other export jobs that use deliver_later, right? There are some using deliver_now and others using deliver_later

@ace
Copy link
Copy Markdown
Contributor Author

ace commented May 25, 2020

@jesusdb It seems some specs on the admin module timed out, can you re-run them?

jesusdb
jesusdb approved these changes May 26, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@jesusdb jesusdb left a comment

Choose a reason for hiding this comment

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

At the time I'm seeing this, all checks have passed 😄

@jesusdb jesusdb merged commit 03c8962 into decidim:develop May 26, 2020
@ace ace deleted the feature/export_initiatives branch May 26, 2020 09:41
ace pushed a commit to aspgems/decidim that referenced this pull request May 27, 2020
Merge branch 'develop' into feature/initiatives-signature-gauge
1d90f06 
* develop:
  Include year in meetings card (decidim#6102)
  Add attachment enabled option to initiative types and initiatives (decidim#6036)
  Fix a flaky test in group profile conversations (decidim#6123)
  Add attachments to Initiatives (decidim#5844)
  Add initiatives export (decidim#6070)
  Improvements to conversations with more than one participant (decidim#6094)
  Elections module and election administration (decidim#6065)
  Separate forms in steps (decidim#6108)
  Add sorting by publishing date to initiatives (decidim#6016)
  Improve proposal preview: Use proposal card when previewing a proposal draft (decidim#6064)
  Newsletter templates fixes (decidim#6096)

# Conflicts:
#	decidim-initiatives/app/models/decidim/initiative.rb
#	decidim-initiatives/spec/system/admin/initiative_types_controller_spec.rb
ace pushed a commit to aspgems/decidim that referenced this pull request May 28, 2020
Merge branch 'feature/add_areas_to_initiatives' into deploy
cb2252c 
* feature/add_areas_to_initiatives: (30 commits)
  Adds areas to FO filters
  Fix lint issue
  Fixes rubocop issues
  Updates changelog
  Adds areas to initiatives
  Send notification when signature threshold reached (decidim#6098)
  Adds filter by initiative type to admin panel (decidim#6093)
  Require confirmation on exiting a survey mid-answering (decidim#6118)
  Information message when there isn't any Proposal (decidim#6063)
  Set email asset host dynamically (decidim#5888)
  Harmonizes the design of initiatives search in FO (decidim#6090)
  Include year in meetings card (decidim#6102)
  Add attachment enabled option to initiative types and initiatives (decidim#6036)
  Fix a flaky test in group profile conversations (decidim#6123)
  Add attachments to Initiatives (decidim#5844)
  Add initiatives export (decidim#6070)
  Improvements to conversations with more than one participant (decidim#6094)
  Elections module and election administration (decidim#6065)
  Separate forms in steps (decidim#6108)
  Add sorting by publishing date to initiatives (decidim#6016)
  ...

# Conflicts:
#	decidim-initiatives/app/cells/decidim/initiatives/initiative_m_cell.rb
#	decidim-initiatives/app/commands/decidim/initiatives/admin/update_initiative.rb
#	decidim-initiatives/app/controllers/decidim/initiatives/initiatives_controller.rb
#	decidim-initiatives/app/forms/decidim/initiatives/admin/initiative_form.rb
#	decidim-initiatives/app/helpers/decidim/initiatives/application_helper.rb
#	decidim-initiatives/app/models/decidim/initiative.rb
#	decidim-initiatives/app/services/decidim/initiatives/initiative_search.rb
#	decidim-initiatives/app/views/decidim/initiatives/create_initiative/fill_data.html.erb
#	decidim-initiatives/app/views/decidim/initiatives/initiatives/_filters.html.erb
#	decidim-initiatives/app/views/decidim/initiatives/initiatives/_tags.html.erb
#	decidim-initiatives/config/locales/en.yml
#	decidim-initiatives/db/migrate/20200514085422_add_area_to_initiatives.rb
#	decidim-initiatives/db/migrate/20200514102631_add_area_enabled_option_to_initiatives.rb
#	decidim-initiatives/spec/forms/initiative_form_spec.rb
#	decidim-initiatives/spec/services/decidim/initiatives/initiative_search_spec.rb
#	decidim-initiatives/spec/shared/update_initiative_type_example.rb
#	decidim-initiatives/spec/system/admin/admin_manages_initiatives_spec.rb
#	decidim-initiatives/spec/system/admin/initiative_types_controller_spec.rb
#	decidim-initiatives/spec/system/filter_initiatives_spec.rb
@sauloperez sauloperez mentioned this pull request Oct 2, 2020
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jesusdb jesusdb jesusdb approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ace @jesusdb @microstudi