Skip to content

Fix traceability logs with invalid record#6879

Merged
Leusev merged 6 commits intodecidim:developfrom
mainio:fix/traceability-log-with-invalid-record
Nov 25, 2020
Merged

Fix traceability logs with invalid record#6879
Leusev merged 6 commits intodecidim:developfrom
mainio:fix/traceability-log-with-invalid-record

Conversation

@ahukkanen
Copy link
Copy Markdown
Contributor

@ahukkanen ahukkanen commented Nov 17, 2020 •
edited
Loading

🎩 What? Why?

When trying to create a new assembly with an image that has too large image dimensions, you will see a server error. This happens because the action logging fails for the record as it was not created and it does not have an ID therefore.

This fixes the issue all along for all records and adds tests particularly for the assemblies where this issue was found from. The server error happened only when assembly was trying to be created. During its update, it did not fail with a server error but the action log was incorrectly added.

This would cause the following kind of exception to appear in the logs:

I, [2020-11-16T13:40:01.265320 #20891] INFO -- : [1234a567-890b-12c3-4def-5ab6cd789e01] Completed 500 Internal Server Error in 250ms (ActiveRecord: 8.7ms)
F, [2020-11-16T13:40:01.267575 #20891] FATAL -- : [1234a567-890b-12c3-4def-5ab6cd789e01]   
F, [2020-11-16T13:40:01.267659 #20891] FATAL -- : [1234a567-890b-12c3-4def-5ab6cd789e01] ActiveRecord::NotNullViolation (PG::NotNullViolation: ERROR:  null value in column "resource_id" violates not-null constraint
DETAIL:  Failing row contains (123, 1, 2, null, Decidim::Assembly, null, null, null, create, {"user": {"ip": "1.2.3.4", "name": "Barry White", "nic..., 2020-11-16 11:40:01.260944, 2020-11-16 11:40:01.260944, null, admin-only, null, null).
: INSERT INTO "decidim_action_logs" ("decidim_organization_id", "decidim_user_id", "resource_type", "action", "extra", "created_at", "updated_at") VALUES ($1, $2, $3, $4, $5, $6, $7) RETURNING "id"):
F, [2020-11-16T13:40:01.267681 #20891] FATAL -- : [1234a567-890b-12c3-4def-5ab6cd789e01]   
F, [2020-11-16T13:40:01.267751 #20891] FATAL -- : [1234a567-890b-12c3-4def-5ab6cd789e01] activerecord (5.2.4.3) lib/active_record/connection_adapters/postgresql_adapter.rb:611:in `async_exec_params'
[... irrelevant parts of the exception cut from here ...]
[1234a567-890b-12c3-4def-5ab6cd789e01] decidim-core (0.22.0) app/services/decidim/traceability.rb:62:in `perform_action!'
[1234a567-890b-12c3-4def-5ab6cd789e01] decidim-core (0.22.0) app/services/decidim/traceability.rb:32:in `create'
[1234a567-890b-12c3-4def-5ab6cd789e01] decidim-assemblies (0.22.0) app/commands/decidim/assemblies/admin/create_assembly.rb:42:in `assembly'
[1234a567-890b-12c3-4def-5ab6cd789e01] decidim-assemblies (0.22.0) app/commands/decidim/assemblies/admin/create_assembly.rb:25:in `call'

Testing

  • Create a Decidim instance with the default seed data
  • Go to assemblies
  • Go to create a new assembly
  • Fill all required fields
  • For either of the image (banner or hero), add an image that has image dimensions more than the maximum configured dimensions, e.g. 5000x5000 (example included in this PR)

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • ❓ CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • ❌AVOID breaking the continuous integration build.
  • ❌AVOID making significant changes to the overall architecture.

@Leusev Leusev self-requested a review November 25, 2020 07:54
@Leusev Leusev self-assigned this Nov 25, 2020
Leusev
Leusev approved these changes Nov 25, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@Leusev Leusev left a comment

Choose a reason for hiding this comment

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

Good catch @ahukkanen 🎉
Thanks for your work!

@Leusev Leusev merged commit 8ecd579 into decidim:develop Nov 25, 2020
@ahukkanen ahukkanen deleted the fix/traceability-log-with-invalid-record branch November 25, 2020 08:07
ahukkanen added a commit to mainio/decidim that referenced this pull request Nov 25, 2020
@ahukkanen
Fix traceability logs with invalid record (decidim#6879)
faf35ed 
* Prevent traceability action log for invalid records

* Add test for creating invalid records with traceability

* Add a spec testing creating assembly with too large image dimensions

* Add a spec testing updating assembly with too large image dimensions

* Add a test image with large image dimensions to fail uploader checks

* Enable checking the hero image error message for the create spec
@ahukkanen ahukkanen mentioned this pull request Nov 25, 2020
Merged
12 tasks
tramuntanal pushed a commit that referenced this pull request Nov 25, 2020
@ahukkanen
Fix traceability logs with invalid record (#6879) (#6928)
a39c7ae 
* Prevent traceability action log for invalid records

* Add test for creating invalid records with traceability

* Add a spec testing creating assembly with too large image dimensions

* Add a spec testing updating assembly with too large image dimensions

* Add a test image with large image dimensions to fail uploader checks

* Enable checking the hero image error message for the create spec
entantoencuanto added a commit that referenced this pull request Dec 2, 2020
@entantoencuanto
Merge branch 'develop' into feature/process_groups_improvements-meeti…
844f4cd 
…ngs_content_block

* develop: (22 commits)
  Fix email CTA alignment on Outlook and Windows Mail (#6895)
  Fix mailer meeting registration invitation using path instead of URL (#6965)
  Fix the data portability exporter when zip is not in the gemfile (#6969)
  Convert technical docs to Antora (#6526)
  New Crowdin updates (#6957)
  Bugfix - moderated meetings are displayed in the meetings index page  (#6927)
  Add HTML Content Blocks in Process Groups  (#6823)
  Prevent error in view due to optional html not showing (#6942)
  Improve layout for standalone T&C page (#6944)
  chore: move rubocop ruby config to own file (#6952)
  Fix some strings (#6958)
  Fix newsletter html containing style tag content (#6876)
  New Crowdin updates (#6945)
  New Crowdin updates (#6926)
  Localize a string in conference speaker (#6866)
  Fix broken dashboard action logs under certain conditions (#6857)
  Fix traceability logs with invalid record (#6879)
  Allow user to drag address on proposal map (#6291)
  New Crowdin updates (#6898)
  Update release notes documentation (#6809)
  ...
entantoencuanto added a commit that referenced this pull request Dec 2, 2020
@entantoencuanto
Merge branch 'develop' into feature/process_groups_improvements-stats…
16b6a73 
…_content_block

* develop: (22 commits)
  Fix email CTA alignment on Outlook and Windows Mail (#6895)
  Fix mailer meeting registration invitation using path instead of URL (#6965)
  Fix the data portability exporter when zip is not in the gemfile (#6969)
  Convert technical docs to Antora (#6526)
  New Crowdin updates (#6957)
  Bugfix - moderated meetings are displayed in the meetings index page  (#6927)
  Add HTML Content Blocks in Process Groups  (#6823)
  Prevent error in view due to optional html not showing (#6942)
  Improve layout for standalone T&C page (#6944)
  chore: move rubocop ruby config to own file (#6952)
  Fix some strings (#6958)
  Fix newsletter html containing style tag content (#6876)
  New Crowdin updates (#6945)
  New Crowdin updates (#6926)
  Localize a string in conference speaker (#6866)
  Fix broken dashboard action logs under certain conditions (#6857)
  Fix traceability logs with invalid record (#6879)
  Allow user to drag address on proposal map (#6291)
  New Crowdin updates (#6898)
  Update release notes documentation (#6809)
  ...
entantoencuanto added a commit that referenced this pull request Dec 2, 2020
@entantoencuanto
Merge branch 'develop' into feature/process_groups_improvements-parti…
6950976 
…cipatory_processes_content_block

* develop: (22 commits)
  Fix email CTA alignment on Outlook and Windows Mail (#6895)
  Fix mailer meeting registration invitation using path instead of URL (#6965)
  Fix the data portability exporter when zip is not in the gemfile (#6969)
  Convert technical docs to Antora (#6526)
  New Crowdin updates (#6957)
  Bugfix - moderated meetings are displayed in the meetings index page  (#6927)
  Add HTML Content Blocks in Process Groups  (#6823)
  Prevent error in view due to optional html not showing (#6942)
  Improve layout for standalone T&C page (#6944)
  chore: move rubocop ruby config to own file (#6952)
  Fix some strings (#6958)
  Fix newsletter html containing style tag content (#6876)
  New Crowdin updates (#6945)
  New Crowdin updates (#6926)
  Localize a string in conference speaker (#6866)
  Fix broken dashboard action logs under certain conditions (#6857)
  Fix traceability logs with invalid record (#6879)
  Allow user to drag address on proposal map (#6291)
  New Crowdin updates (#6898)
  Update release notes documentation (#6809)
  ...
entantoencuanto added a commit that referenced this pull request Dec 2, 2020
@entantoencuanto
Merge branch 'develop' into feature/process_groups_improvements-edit_…
a401ade 
…link

* develop: (22 commits)
  Fix email CTA alignment on Outlook and Windows Mail (#6895)
  Fix mailer meeting registration invitation using path instead of URL (#6965)
  Fix the data portability exporter when zip is not in the gemfile (#6969)
  Convert technical docs to Antora (#6526)
  New Crowdin updates (#6957)
  Bugfix - moderated meetings are displayed in the meetings index page  (#6927)
  Add HTML Content Blocks in Process Groups  (#6823)
  Prevent error in view due to optional html not showing (#6942)
  Improve layout for standalone T&C page (#6944)
  chore: move rubocop ruby config to own file (#6952)
  Fix some strings (#6958)
  Fix newsletter html containing style tag content (#6876)
  New Crowdin updates (#6945)
  New Crowdin updates (#6926)
  Localize a string in conference speaker (#6866)
  Fix broken dashboard action logs under certain conditions (#6857)
  Fix traceability logs with invalid record (#6879)
  Allow user to drag address on proposal map (#6291)
  New Crowdin updates (#6898)
  Update release notes documentation (#6809)
  ...
entantoencuanto added a commit that referenced this pull request Dec 2, 2020
@entantoencuanto
Merge branch 'develop' into feature/process_groups_improvements-highl…
6c9ebfe 
…ighted_groups

* develop: (22 commits)
  Fix email CTA alignment on Outlook and Windows Mail (#6895)
  Fix mailer meeting registration invitation using path instead of URL (#6965)
  Fix the data portability exporter when zip is not in the gemfile (#6969)
  Convert technical docs to Antora (#6526)
  New Crowdin updates (#6957)
  Bugfix - moderated meetings are displayed in the meetings index page  (#6927)
  Add HTML Content Blocks in Process Groups  (#6823)
  Prevent error in view due to optional html not showing (#6942)
  Improve layout for standalone T&C page (#6944)
  chore: move rubocop ruby config to own file (#6952)
  Fix some strings (#6958)
  Fix newsletter html containing style tag content (#6876)
  New Crowdin updates (#6945)
  New Crowdin updates (#6926)
  Localize a string in conference speaker (#6866)
  Fix broken dashboard action logs under certain conditions (#6857)
  Fix traceability logs with invalid record (#6879)
  Allow user to drag address on proposal map (#6291)
  New Crowdin updates (#6898)
  Update release notes documentation (#6809)
  ...
entantoencuanto added a commit that referenced this pull request Dec 2, 2020
@entantoencuanto
Merge branch 'develop' into feature/process_groups_improvements-group…
f0066f7 
…s_and_processes_block

* develop: (22 commits)
  Fix email CTA alignment on Outlook and Windows Mail (#6895)
  Fix mailer meeting registration invitation using path instead of URL (#6965)
  Fix the data portability exporter when zip is not in the gemfile (#6969)
  Convert technical docs to Antora (#6526)
  New Crowdin updates (#6957)
  Bugfix - moderated meetings are displayed in the meetings index page  (#6927)
  Add HTML Content Blocks in Process Groups  (#6823)
  Prevent error in view due to optional html not showing (#6942)
  Improve layout for standalone T&C page (#6944)
  chore: move rubocop ruby config to own file (#6952)
  Fix some strings (#6958)
  Fix newsletter html containing style tag content (#6876)
  New Crowdin updates (#6945)
  New Crowdin updates (#6926)
  Localize a string in conference speaker (#6866)
  Fix broken dashboard action logs under certain conditions (#6857)
  Fix traceability logs with invalid record (#6879)
  Allow user to drag address on proposal map (#6291)
  New Crowdin updates (#6898)
  Update release notes documentation (#6809)
  ...
@mrcasals mrcasals added type: fix PRs that implement a fix for a bug and removed type: bug labels Feb 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@Leusev Leusev Leusev approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@Leusev Leusev

Labels

in-review module: assemblies module: core type: fix PRs that implement a fix for a bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ahukkanen @Leusev @mrcasals