Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ActionView::Template::Error: Couldn't find User #3321

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

vedant-jain03
Copy link
Member

Fixes #3317

Describe the changes you have made in this PR -

  • Added data migration
  • Added noticed_notification association with user.

Screenshots of the changes (If any) -

Note: Please check Allow edits from maintainers. if you would like us to assist in the PR.

@vedant-jain03 vedant-jain03 added the priority Needs to be resolved or completed as soon as possible label Oct 16, 2022
@vedant-jain03 vedant-jain03 self-assigned this Oct 16, 2022
@coveralls
Copy link

coveralls commented Oct 16, 2022

Coverage Status

Coverage increased (+0.007%) to 82.041% when pulling 70c7d7e on vedant-jain03:issue#3317 into d27a74b on CircuitVerse:master.

@vedant-jain03 vedant-jain03 changed the title Issue#3317 fix: ActionView::Template::Error: Couldn't find User Oct 16, 2022
@vedant-jain03
Copy link
Member Author

cc @tachyons, kindly review

Comment on lines +31 to 32
has_noticed_notifications model_name: "NoticedNotification"
has_many :noticed_notifications, as: :recipient, dependent: :destroy
Copy link
Member

Choose a reason for hiding this comment

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

Do we need both lines ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes,
According to the noticed doc has_noticed_notifications should cover that edge, but I tested it for the project as well, it is giving unexpected results. So we can move ahead with the standard association method for now.

…n_existing_notifier_user.rb

Co-authored-by: Aboobacker MK <aboobackervyd@gmail.com>
@codeclimate
Copy link

codeclimate bot commented Nov 3, 2022

An error occurred when fetching issues.

View more on Code Climate.

@vedant-jain03 vedant-jain03 added this to Ready for Review in Main Platform Improvement Nov 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority Needs to be resolved or completed as soon as possible
Projects
Main Platform Improvement
Need / Ready for Review
Development

Successfully merging this pull request may close these issues.

ActionView::Template::Error: Couldn't find User with 'id'=148788
3 participants