Skip to content

Check if audits is loaded before executing database queries#715

Open
wkirby wants to merge 5 commits intocollectiveidea:mainfrom
apsislabs:apsis/loaded-audits
Open

Check if audits is loaded before executing database queries#715
wkirby wants to merge 5 commits intocollectiveidea:mainfrom
apsislabs:apsis/loaded-audits

Conversation

@wkirby
Copy link

@wkirby wkirby commented Jun 13, 2024

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

@danielmorrison
Copy link
Member

Tests are failing, but I like the idea.

@wkirby
Copy link
Author

wkirby commented Aug 24, 2024

@danielmorrison thanks, will fix test failures

@wkirby wkirby force-pushed the apsis/loaded-audits branch from 48088fc to b7f99e8 Compare August 24, 2024 12:56
@wkirby wkirby force-pushed the apsis/loaded-audits branch from b7f99e8 to 1b72a97 Compare August 24, 2024 13:37
@wkirby
Copy link
Author

wkirby commented Aug 24, 2024

@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
Copy link
Author

wkirby commented Oct 16, 2024

@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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Preloading audits is ignored when accessing specific revision

2 participants