Skip to content

Set email asset host dynamically#5888

Merged
jesusdb merged 8 commits intodecidim:developfrom
armandfardeau:feature/set-email-asset-host
May 27, 2020
Merged

Set email asset host dynamically#5888
jesusdb merged 8 commits intodecidim:developfrom
armandfardeau:feature/set-email-asset-host

Conversation

@armandfardeau
Copy link
Copy Markdown
Contributor

@armandfardeau armandfardeau commented Mar 23, 2020

🎩 What? Why?

Set emails asset host dynamically.

It can be strange for users to find resource URLs that are linked to another organization, this development allows to either define a common asset host for all organizations (example: assets.decidim.org) or to reroute assets according to the organization's host.

📋 Subtasks

  • Add CHANGELOG entry
  • Add tests

@armandfardeau armandfardeau marked this pull request as ready for review March 23, 2020 13:38
@jesusdb
Copy link
Copy Markdown
Contributor

jesusdb commented Apr 27, 2020

Hello @armandfardeau, has this been approved by @decidim/product?

@virgile-dev
Copy link
Copy Markdown
Contributor

virgile-dev commented Apr 27, 2020

@carolromero this is ready to be reviewed :)
It's super handy for multi-tenant

@armandfardeau
Copy link
Copy Markdown
Contributor Author

Hello @armandfardeau, has this been approved by @decidim/product?

Not yet !

Copy link
Copy Markdown
Contributor

@tramuntanal tramuntanal left a comment

Choose a reason for hiding this comment

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

Good contribution @armandfardeau I just have a question and share the same with @microstudi

@tramuntanal tramuntanal self-assigned this Apr 29, 2020
@tramuntanal
Copy link
Copy Markdown
Contributor

Hi @decidim/product , can you please confirm this PR? I find it interesting

@carolromero
Copy link
Copy Markdown
Member

@tramuntanal I don't understand the purpose of this feature. Could you give an example of what solves this? Thanks!

@microstudi
Copy link
Copy Markdown
Contributor

@tramuntanal I don't understand the purpose of this feature. Could you give an example of what solves this? Thanks!

When you receive and email from a tenant instead of the main Decidim platform, any image or asset linked in the email will show the URL of the main instance instead of the tenant. Most people won't notice it, but it is healthy to be consistent.

@carolromero
Copy link
Copy Markdown
Member

thanks for the explanation @microstudi, all clear now, and thanks for the contribution @armandfardeau

@tramuntanal
Copy link
Copy Markdown
Contributor

@armandfardeau it's approved, can you apply changes from reviews and we're done please?

microstudi
microstudi previously approved these changes May 25, 2020
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.

Thanks for the contribution! 😄

@jesusdb jesusdb merged commit 5f382c7 into decidim:develop May 27, 2020
@mrcasals
Copy link
Copy Markdown
Contributor

Hi! After merging this PR, develop is showing errors. Can you check what's going on?

https://github.com/decidim/decidim/runs/712419406

@jesusdb
Copy link
Copy Markdown
Contributor

jesusdb commented May 27, 2020

I'm sorry, I could swear I saw all tests passing. I just re-run the test, if it fails, I will revert it

@mrcasals
Copy link
Copy Markdown
Contributor

Maybe the tests passed, but got a weird error after #6096 and the branch in this PR was not updated, so it was not detected? 🤔

ace pushed a commit to aspgems/decidim that referenced this pull request May 28, 2020
* 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
ace pushed a commit to aspgems/decidim that referenced this pull request May 29, 2020
* develop:
  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)
@armandfardeau armandfardeau deleted the feature/set-email-asset-host branch May 29, 2020 15:36
anaghavl pushed a commit to codegram/decidim that referenced this pull request Jun 3, 2020
* Set email asset host dynamically

* Add changelog entry

* Use decidim force_ssl configuration accessor

* Fix typo
@tramuntanal tramuntanal mentioned this pull request Jul 30, 2020
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants