Issue preloading soft deleted records

[jwoertink]

I'll need to investigate this more, but we just ran in to an issue with some preloads.

Take this model setup

class User < BaseModel
  table do
    # ...
    column soft_deleted_at : Time?
    has_many transactions : Transaction
  end
end

class Transaction < BaseModel
  table do
    belongs_to user : User
  end
end

class User::BaseQuery
  include Avram::SoftDelete::Query

  def initialize
    defaults &.only_kept
  end
end

class UserQuery < User::BaseQuery
end

class TransactionQuery < Transaction::BaseQuery
end

Then we had some code like this:

# in the action
transactions = TransactionQuery.new.preload_user

# in the page

transactions.each do |transaction|
  td do
    link(transaction.user.username, to: Users::Show.with(transaction.user_id))
  end
end

We kept getting a 500 error saying NilAssertionError which generally relates to calling not_nil! somewhere. In this case, our user should never be nil, but calling transaction.user says otherwise. We printed out the record, and saw this:

#<Transaction:0x7f9cdd05a8f0 @_user_preloaded=true, @_preloaded_user=nil, @id=1, @created_at=2021-04-28 18:02:47.0 UTC, @updated_at=2021-04-28 18:02:47.0 UTC, @user_id=4>

Looking up user 4, we saw the record existed, but it was soft deleted.

Avram should probably log some sort of warning that a required record could not be preloaded, or maybe when you preload an association that's been soft deleted, that record isn't returned at all?

[matthewmcgarvey]

Another option would be to raise an error. Transaction specifies that user association will be there but it isn't so something is wrong

[jwoertink]

Yeah, that's a good idea. It took a long time to figure out that error, so throwing an error that's easier to spot what's going on would make things a lot nicer.

AND.... to get around this, the proper query was

transactions = TransactionQuery.new.preload_user.where_user(UserQuery.new)

So maybe when we raise the error, we can mention something like that.

[jwoertink]

Just ran in to this again, and it it gets a bit more complicated.

class User < BaseModel
  table do
    # ...
    has_many following_follows : Follow, foreign_key: :follower_id
    has_many follower_follows : Follow, foreign_key: :followee_id
    has_many followers : User, through: [:follower_follows, :follower]
    has_many followings : User, through: [:following_follows, :followee] 
  end
end

class Follow < BaseModel
  include Avram::SoftDelete::Model

  table do
    column soft_deleted_at : Time?
    belongs_to follower : User, foreign_key: :follower_id
    belongs_to followee : User, foreign_key: :followee_id
  end
end

I have this sort of model setup. When someone follows a user then unfollows, we just soft delete that "follow" record so they can re-follow again. However, if they are unfollowed and that soft_deleted_at is set, we start to run in to issues with preloads...

UserQuery.new.preload_followers

This will throw a NilAssertionError error. Unlike my previous comment, I can't just preload_followers.where_followers() because "followers" are User... I need to have it sub-preload the follower_follows.... or, you know, figure out how to fix this :joy:

In this case, I'm opting to just not preload anything until I can figure out a solution

[matthewmcgarvey]

I haven't thought this through, but a thought I had was to add methods that return the query object for each of the associations so that they can be overridden. Still think we should raise an error on belongs_to associations not being found, though.

[matthewmcgarvey]

Looking at this again, there's a query problem, yes, but the real problem seems to be the data integrity. When a user is (soft) deleted, it makes sense that their follows are also (soft) deleted. At the database level we require an on_delete strategy, I'm wondering what it would look like to adding on_soft_delete to Avram::SoftDelete::Model 🤔

[jwoertink]

I think you're right, and I kinda like the idea of that callback for on_soft_delete. The tricky part becomes when you have a huge eco system of models interconnected. When you soft delete a user, then you're like "ok, let's also soft delete EVERY associated record".... Though, in our case, the follow records can actually be real deleted.

Here's how our setup is. Maybe this use case will help to think out what would need to be done..

We have a Subscription model, and a Transaction model. The Transaction is the record of a purchase with a credit card that's been made. The Subscription is the record that keeps track of associating one user that subscribes to another user, how much, and how many times they've re-subscribed. When a user a deleted, we still need to know about the subscription and the transaction so we can handle doing revshare, and reporting. We can't really "soft" delete these either because the user that got the subscription still needs to see that record for their own reporting. So we soft delete the user and basically anonymize it. The follow is irrelevant, so that can be fully deleted, but the subscription and transaction can't be deleted or even soft deleted really...

Then we run in to the issue when we want to display all of the subscriptions a user got

# This fails when one of the users being preloaded is soft deleted
SubscriptionQuery.new.streamer_id(current_user.id).preload_user

The good thing is there's at least some "hacks" around it, but it doesn't really follow the "Lucky way" since I usually end up catching this when it's in production.

hmm... If I call preload_user, and we know that User is soft deletable... could that raise a compile time error saying something like "You need to pass a query object in" or whatever? Still just thinking this out, but would there be a way to just catch that so I'm sort of forced to do SubscriptionQuery.new.preload_user(UserQuery.new.with_soft_deleted) or whatever? :thinking:

[jwoertink]

This actually may not really be an issue. You can do this.

SubscriptionQuery.new.streamer_id(current_user.id).preload_user(&.with_soft_deleted)