audited icon indicating copy to clipboard operation
audited copied to clipboard

Check if audits is loaded before executing database queries

Open wkirby opened this issue 1 year ago • 4 comments

This should be a no-op change that allows for performance improvements by preloading the audits association. By checking if the audits association is already loaded, we can avoid N+1 queries by operating on the already loaded audits in ruby instead of going to the database again to check if a version exists, etc.

This code should be functionally identical to the existing code, but allows for something like:

@posts = Post.all.include(:audits)

@posts_at_v1 = @posts.map do |post|
  post.revision(1)
end

To avoid repeated database access.

Fixes #719

wkirby avatar Jun 13 '24 20:06 wkirby

Tests are failing, but I like the idea.

danielmorrison avatar Jul 30 '24 14:07 danielmorrison

@danielmorrison thanks, will fix test failures

wkirby avatar Aug 24 '24 11:08 wkirby

@danielmorrison specs are now passing except for code coverage (see the test run on our fork here: https://github.com/apsislabs/audited/actions/runs/10538941999?pr=1). I believe this is because of the addition of the new loaded? branches being untested.

I'm happy to either:

  • Update coverage exceptions so the tests pass
  • Attempt to add some specs to cover these branches

wkirby avatar Aug 24 '24 14:08 wkirby

@danielmorrison just thought I'd poke in here and see if you had an answer for how you wanted me to handle the new untested lines?

wkirby avatar Oct 16 '24 17:10 wkirby