Newsletter templates fixes #6096
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@decidim/core please, could you merge this PR to prevent this problem? Here are some examples of admin tests failing after 6 hours running: |
|
@decidim/core we are still having GitHub Actions tasks that are taking 6 hours to fail (you can check it here). I think this PR solves that, can you review it? |
ace
pushed a commit
to aspgems/decidim
that referenced
this pull request
May 27, 2020
* 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
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
馃帺 What? Why?
Newsletter templates tests are randomly failing and making the admin component tests to take more than 5 hours to finish. After investigating a lot about that, I didn't find the exact reason for this problem, but it seems to be related with errors when autoloading modules. What I did find is the exact event flow that produces it: when loading the previews for the two templates at the same time the
Decidim::ViewModelloads three modules that make the server to hang up (Decidim::ActionAuthorization,Decidim::ActionAuthorizationHelperandDecidim::ResourceHelper). This happens in testing and development mode as eager load is disabled.To fix the problem, I've moved the
Decidim::ActionAuthorizationmodule to thelibfolder and addedautoloadstatement for those modules.During the debugging session I've also discovered that the Newsletter's template could be memoized to avoid creating unnecesary ContentBlock, and that templates preview design were broken when the container iframe is too small. So this PR also fix those minor issues.
馃搶 Related Issues
馃搵 Subtasks
CHANGELOGentry馃摲 Screenshots (optional)
The minor design issue fixed by this PR:
