Feat/skipfilters optional list #657
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: jakeb994 <jakeb994@gmail.com>
Co-authored-by: jakeb994 <jakeb994@gmail.com>
Co-authored-by: jakeb994 <jakeb994@gmail.com>
…tion safety Co-authored-by: jakeb994 <jakeb994@gmail.com>
Contributor
WalkthroughAdds per-filter skipping to Database: a protected Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Database
participant Filters
Caller->>Database: skipFilters(callback, filterNames = null)
alt No specific filters
Database->>Database: Save current filter state
Database->>Filters: Disable all filters
Database->>Database: Execute callback
Database->>Filters: Restore all filters
else Specific filters list
Database->>Database: Save filter state + disabledFilters
Database->>Database: Set disabledFilters = {names}
Database->>Database: Execute callback
Database->>Database: Restore filter state + disabledFilters
end
sequenceDiagram
participant Caller
participant Database
participant FilterX
Caller->>Database: decodeAttribute(attr, value)
Database->>Database: Check disabledFilters for FilterX
alt FilterX disabled
Database-->>Caller: return value (unchanged)
else Active
Database->>FilterX: decode(value)
FilterX-->>Database: decodedValue
Database-->>Caller: decodedValue
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/Database/Database.php (3)
351-352: Tighten type and clarify semantics for disabledFiltersCurrent nullable type isn’t used (never set to null). Prefer a non-nullable map with clear key/value intent.
- protected ?array $disabledFilters = []; + /** @var array<string, bool> map of filter-name => true to disable selectively */ + protected array $disabledFilters = [];
820-852: Refine skipFilters: remove unnecessary state handling and simplify map build
- Capturing/restoring $this->filter in the selective branch is unnecessary (it’s never changed there).
- Build the disabled map via array_fill_keys for clarity.
- Consider documenting the new parameter in the docblock.
- public function skipFilters(callable $callback, ?array $filterNames = null): mixed + public function skipFilters(callable $callback, ?array $filterNames = null): mixed { if (\is_null($filterNames)) { $initial = $this->filter; $this->disableFilters(); try { return $callback(); } finally { $this->filter = $initial; } } - $previousFilter = $this->filter; $previousDisabledFilters = $this->disabledFilters; - if (\count($filterNames) === 0) { - $this->disabledFilters = []; - } else { - $disabledFilters = []; - foreach ($filterNames as $filterName) { - $disabledFilters[$filterName] = true; - } - $this->disabledFilters = $disabledFilters; - } + // Empty list => disable none (empty map) + $this->disabledFilters = \array_fill_keys($filterNames, true) ?: []; try { return $callback(); } finally { - $this->filter = $previousFilter; $this->disabledFilters = $previousDisabledFilters; } }Docblock nit (outside changed lines): please add
- “@param array|null $filterNames Optional filter names to disable; null disables all filters.”
6652-6655: Simplify disabled-filter check and align with non-nullable mapSince disabledFilters is an array (see suggestion above), the is_null check can be dropped.
- if (!\is_null($this->disabledFilters) && isset($this->disabledFilters[$filter])) { + if (isset($this->disabledFilters[$filter])) { return $value; }Also, confirm intent: selective skipping currently applies only to decode. If you want parity for encode as well, we can add the same check in encodeAttribute.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Database/Database.php(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Database/Database.php (1)
src/Database/Adapter.php (1)
filter(1127-1136)
abnegate
added a commit
that referenced
this pull request
Aug 11, 2025
Feat/skipfilters optional list
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes