Skip to content

feat(query): ignore system attributes _id/_time/_version when matching#466

Open
SAY-5 wants to merge 1 commit into
zerocracy:masterfrom
SAY-5:fix/except-symbol-keys
Open

feat(query): ignore system attributes _id/_time/_version when matching#466
SAY-5 wants to merge 1 commit into
zerocracy:masterfrom
SAY-5:fix/except-symbol-keys

Conversation

@SAY-5
Copy link
Copy Markdown

@SAY-5 SAY-5 commented May 19, 2026

Closes #460. The except call in if_absent and just_one used string keys while attrs is keyed by symbols, so the system attributes were never stripped from the match query. This passes symbols to except and adds regression tests for both helpers.

@0crat
Copy link
Copy Markdown

0crat commented May 19, 2026

@SAY-5 Hi there! Your branch name "fix/except-symbol-keys" doesn't follow our naming convention, so I've deducted -6 points as per our team policy. Please use the ticket number as the branch name next time - in this case, "465" would be perfect. Your current score is -6, and you can check your Zerocracy account for updates.

Copy link
Copy Markdown

@edmoffo edmoffo left a comment

Choose a reason for hiding this comment

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

Diff matches the analysis in #460: attrs carries symbol keys throughout, so passing symbols to except removes the system attributes that the docstrings already promised would be skipped. Regression tests cover both if_absent and just_one, and CI is green.

Copy link
Copy Markdown
Contributor

@bibonix bibonix left a comment

Choose a reason for hiding this comment

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

The change in both if_absent.rb and just_one.rb passes symbol keys (:_id, :_time, :_version) to Hash#except instead of strings, which matches the symbol-keyed attrs hash and actually strips the system attributes from the match query as the docstrings already promised. The two regression tests cover both helpers and both end up at the right place: if_absent returns nil when only the system attribute differs, and just_one returns the existing fact. CI is green.

@0crat
Copy link
Copy Markdown

0crat commented May 20, 2026

@bibonix Hey! Nice work on that review 👍 You snagged +10 points this time: started with the base +18 but lost -8 since no comments were posted during the review. Just so you know, you could've maxed out at +24 points if there were more comments and hits-of-code involved - something to keep in mind for next time! Your running score is now +400, so don't forget to check your Zerocracy account too.

@0crat
Copy link
Copy Markdown

0crat commented May 20, 2026

@edmoffo Great work on the review! 🎉 You've earned +10 points (+18 base, -8 for missing comments). Your current score is +70 - keep building momentum! Check your Zerocracy account for more details.

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.

Fbe.if_absent and Fbe.just_one don't actually skip _id/_time/_version in the match query

4 participants