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 notifications when the projects or users are deleted #4904

Open
tachyons opened this issue Apr 20, 2024 · 8 comments · May be fixed by #4910, #4917 or #4947
Open

Fix notifications when the projects or users are deleted #4904

tachyons opened this issue Apr 20, 2024 · 8 comments · May be fixed by #4910, #4917 or #4947
Assignees
Labels

Comments

@tachyons
Copy link
Member

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:

  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
@tachyons tachyons added the good first issue Good for newcomers label Apr 20, 2024
@Malavi1
Copy link
Contributor

Malavi1 commented Apr 21, 2024

Hi @tachyons I am interested in work on this issue.

@Harry-kp
Copy link

Harry-kp commented Apr 21, 2024

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?

@tachyons
Copy link
Member Author

@Harry-kp Issue happens when the project or user id deleted after the notification record is created. So that code snippet is fine.

@Harry-kp
Copy link

@tachyons Understood. Let me know if it would be okay for me to work on this issue as it has been already assigned.

@tachyons
Copy link
Member Author

tachyons commented Apr 21, 2024 via email

@adarsh7raj
Copy link

can you give me steps to reproduce this issue?

@Asrani-Aman
Copy link
Member

I also want to work on this issue

@subhamkumarr
Copy link
Contributor

@Asrani-Aman Malavi and Harry are already assigned

NirvedIITBHU pushed a commit to NirvedIITBHU/CircuitVerse that referenced this issue Apr 23, 2024
@NirvedIITBHU NirvedIITBHU linked a pull request Apr 23, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment