Skip to content

Conversation

@fuzzyrichie
Copy link
Contributor

@fuzzyrichie fuzzyrichie commented Nov 19, 2024

Summary

The intent of this PR is to enable field counters and create two helpers: query_by_field and query_by_field_at_least. When trying to analyze changes to models, we may want to check if there's expected changes by looking for specific fields. In doing so, we can use this to test for changes by using the helpers above:

  • query_by_field matches the changes in fields to the expected changes exactly. If they do not match, the test will fail.
  • query_by_field_at_least matches the changes in fields to the expected changes, but only matches the fields the expected changes have mentioned. For example, if fields A and B are changed but we only expect field A to change, if the actual values and the expected values for A are the same, we'll pass this test - regardless of what field B changes have been made, if any.
  • query_by_field_at_least_ignore_notfound matches the changes in fields to the expected changes, similarly to query_by_field_at_least, but when checking for changes to field A, if the expected changes are included in the actual changes but not the same, we will pass this test. For example, if field A is expected to contain the values [1,2] and the actual values are [1,2,3], query_by_field_at_least_ignore_notfound will succeed but query_by_field_at_least would fail.

tenshiemi
tenshiemi previously approved these changes Apr 29, 2025
Copy link

@tenshiemi tenshiemi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, left some minor comments

# @mission Infrastructure
# @team DEx

require 'logger'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you still need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, unfortunately there's a requirement from active_record in pre-Rails 7.1 versions :/

# - Anything with ` {field} = ` (this could be a select, update, delete)
# - Anything with `, field,` in an INSERT (we need to check the values)
select_field_query = sql.match(MODEL_FIELDS_PATTERN)
# debugger if sql.match(/INSERT/)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this debugger line needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now it can stay, I do need to clean up the TODO.

@fuzzyrichie
Copy link
Contributor Author

@tenshiemi - mind re-approving this one if possible? 😄

@fuzzyrichie fuzzyrichie merged commit 5f7098e into master Apr 29, 2025
4 checks passed
@fuzzyrichie fuzzyrichie deleted the add-field-counter branch April 29, 2025 18:12
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.

3 participants