CircuitVerse
CircuitVerse copied to clipboard
Fix notifications when the projects or users are deleted
When a project is forked, a notification is generated. But if that project is deleted, error is displayed since the notification unable to find the notification.
Solution:
Use null object pattern
Ref:
- https://dev.to/daviducolo/exploring-the-null-object-pattern-in-ruby-k5l
- https://thoughtbot.com/blog/rails-refactoring-example-introduce-null-object
def message
- user = params[:user]
- project = params[:project]
+ user = params[:user] || DeletedUser.new
+ project = params[:project] || DeletedUser.new
t("users.notifications.fork_notification", user: user.name, project: project.name)
end
Hi @tachyons I am interested in work on this issue.
HI @tachyons , If I am not wrong, the notification is triggered from the below piece of code
def fork(user)
forked_project = dup
forked_project.build_project_datum.data = project_datum&.data
forked_project.circuit_preview.attach(circuit_preview.blob)
forked_project.image_preview = image_preview
forked_project.update!(
view: 1, author_id: user.id, forked_project_id: id, name: name
)
@project = Project.find(id)
if @project.author != user # rubocop:disable Style/IfUnlessModifier
ForkNotification.with(user: user, project: @project).deliver_later(@project.author)
end
forked_project
end
Since, find is used, the exception will be raised, ActiveRecord::RecordNotFound for deleted project and the notification will not be triggered.
To fix this issue, should we have to handle the above case also, or am I missing something?
@Harry-kp Issue happens when the project or user id deleted after the notification record is created. So that code snippet is fine.
@tachyons Understood. Let me know if it would be okay for me to work on this issue as it has been already assigned.
You can pair with Malavi
On Sun, 21 Apr, 2024, 2:13 pm Harshit Chaudhary, @.***> wrote:
@tachyons https://github.com/tachyons Understood. Let me know if it would be okay for me to work on this issue as it has been already assigned.
— Reply to this email directly, view it on GitHub https://github.com/CircuitVerse/CircuitVerse/issues/4904#issuecomment-2067962915, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXYAEGJRPOAIAHSY5AL2DTY6N34LAVCNFSM6AAAAABGQTRBNOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRXHE3DEOJRGU . You are receiving this because you were mentioned.Message ID: @.***>
can you give me steps to reproduce this issue?
I also want to work on this issue
@Asrani-Aman Malavi and Harry are already assigned
hey, i just fixed the issue please review