excid3 / noticed

Notifications for Ruby on Rails applications
MIT License
2.4k stars 170 forks source link

How to handle deletion on other relation #252

Closed dvodvo closed 2 years ago

dvodvo commented 2 years ago

The readMe provides a few mechanisms to delete associated records.

class Post < ApplicationRecord
  # Standard association for deleting notifications when you're the recipient
  has_many :notifications, as: :recipient, dependent: :destroy

However, I have a NewPostNotification with param :post and the relation instead is based on

class User < ApplicationRecord
  has_many :notification, as: :recipient

class Post < ApplicationRecord
  has_noticed_notifications

which, when launching on a Post object @post.destroy hits and internal error relation "notifications" does not exist So, in a sense, the application knows there is a notification object associated to the post, but is unaware of the relation based on standard rails functions

How should one go about destroying this object? With console User.last.notifications is an Object that does not support #inspect so there is no obvious way about this.

excid3 commented 2 years ago

The recipient is the only one you can use a has_many on because it's a polymorphic relationship.

Records that are in params like post are in json columns, and therefore can't use has_many :notifications.

dvodvo commented 2 years ago

So what would you suggest? If a User has_many :posts that are subject to database notifications, what to do considering a potentialpost.destroy? On the other hand, I never liked the idea of destroying objects, rather make them invisible or disabled...

excid3 commented 2 years ago

Just like in the README:

class Post
  has_noticed_notifications
end

This adds a destroy callback that deletes notifications where the params[:post] == self

dvodvo commented 2 years ago

That is what is set in the Post class, ergo the issue.

the log registers:

Started DELETE "/discussions/1-how-do-i-start/posts/2" for 127.0.0.1 at 2022-08-22 10:10:59 +0200
Processing by Discussions::PostsController#destroy as TURBO_STREAM
  Parameters: {"authenticity_token"=>"[FILTERED]", "discussion_id"=>"1-how-do-i-start", "id"=>"2"}
[...]
  ↳ app/controllers/discussions/posts_controller.rb:64:in `set_discussion'
  Post Load (0.6ms)  SELECT "posts".* FROM "posts" WHERE "posts"."discussion_id" = $1 AND "posts"."id" = $2 LIMIT $3  [["discussion_id", 1], ["id", 2], ["LIMIT", 1]]
  ↳ app/controllers/discussions/posts_controller.rb:68:in `set_post'
  TRANSACTION (0.6ms)  BEGIN
  ↳ app/models/notification.rb:2:in `include'
  Notification Load (2.8ms)  SELECT "notifications".* FROM "notifications" WHERE (params @> '{"post":{"_aj_globalid":"gid://simeone/Post/2"},"_aj_symbol_keys":["post"]}')
  ↳ app/controllers/discussions/posts_controller.rb:48:in `destroy'
  TRANSACTION (0.6ms)  ROLLBACK
  ↳ app/controllers/discussions/posts_controller.rb:48:in `destroy'
Completed 500 Internal Server Error in 276ms (ActiveRecord: 28.8ms | Allocations: 45678)

ActiveRecord::StatementInvalid (PG::UndefinedTable: ERROR:  relation "notifications" does not exist
LINE 1: SELECT "notifications".* FROM "notifications" WHERE (params ...
                                      ^
):

app/controllers/discussions/posts_controller.rb:48:in `destroy'
  Post Load (1.1ms)  SELECT "posts".* FROM "posts" ORDER BY "posts"."id" DESC LIMIT $1  [["LIMIT", 1]]
  Post Load (1.0ms)  SELECT "posts".* FROM "posts" ORDER BY "posts"."id" DESC LIMIT $1  [["LIMIT", 1]]
  Notification Load (1.5ms)  SELECT "notifications".* FROM "notifications" WHERE (params @> '{"post":{"_aj_globalid":"gid://simeone/Post/2"},"_aj_symbol_keys":["post"]}')
excid3 commented 2 years ago

You don't have a notifications table it says.

dvodvo commented 2 years ago

Ah! Misinterpreted the error message. relation "notifications" does not exist usually infers that the model does not specify the relation, yet, has_noticed_notifications was set. The previous element PG::UndefinedTable says no table - which was the case.

[no idea how the migration got lost]