-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HP-2388: add fields same as in RCP #543
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThis change introduces a new search attribute, Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant V as Search View
participant M as BillSearch Model
U->>V: Enters search term for server
V->>M: Submits search query with `server_ilike`
M->>M: Merges default and `server_ilike` attributes
M-->>V: Returns filtered search results
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
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:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/models/BillSearch.php (1)
28-28
: 💡 Verification agent🧩 Analysis chain
Validate rules for the new 'server_ilike' attribute.
The new
server_ilike
attribute has been added to thesearchAttributes()
method, but it's not explicitly included in therules()
method.Please verify if the new
server_ilike
attribute needs to be added to the rules method:
🏁 Script executed:
#!/bin/bash # Check if any rule in the codebase might already apply to server_ilike rg -l "server_ilike.*safe" --type php # Check the parent class rules that might apply to this attribute rg -l "SearchModelTrait.*rules" --type phpLength of output: 117
Action Required: Add a Safe Rule for
server_ilike
in BillSearchAfter verifying with the search results (notably that
src/models/ConsumptionSearch.php
includes a safe rule forserver_ilike
), it appears that the new attribute should be explicitly added to the rules method. Please updatesrc/models/BillSearch.php
by includingserver_ilike
in the safe validation rules (e.g., extend the existing safe array to include'server_ilike'
).
- File:
src/models/BillSearch.php
- Location: In the
rules()
method, alongside the other safe attributes (e.g., add'server_ilike'
to the array)
🧹 Nitpick comments (1)
src/models/BillSearch.php (1)
45-45
: Added 'server_ilike' to searchAttributes.Good addition of the
server_ilike
attribute to support enhanced server search capabilities.Consider adding a corresponding label for
server_ilike
in theattributeLabels()
method to maintain consistency, similar to howservers
has a label defined.public function attributeLabels(): array { return $this->mergeAttributeLabels([ 'servers' => Yii::t('hipanel:finance', 'Servers'), + 'server_ilike' => Yii::t('hipanel:finance', 'Server'), 'object_name_ilike' => Yii::t('hipanel:finance', 'Object name'), 'object_types' => Yii::t('hipanel:finance', 'Object types'), 'type_id' => Yii::t('hipanel:finance', 'Type'), 'client_types' => Yii::t('hipanel:finance', 'Client types'), 'client_tags' => Yii::t('hipanel:finance', 'Client tags'), ]); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/models/BillSearch.php
(1 hunks)src/views/bill/_search.php
(1 hunks)
🔇 Additional comments (1)
src/views/bill/_search.php (1)
77-77
: Field updated from 'servers' to 'server_ilike'.The search field has been updated to use
server_ilike
instead ofservers
, which aligns with the new search attribute added to the BillSearch model. This change enhances the server search capability by supporting case-insensitive partial matching.
@@ -74,7 +74,7 @@ | |||
|
|||
|
|||
<div class="col-md-4 col-sm-6 col-xs-12"> | |||
<?= $search->field('servers') ?> | |||
<?= $search->field('server_ilike') ?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will include nicX devices, I'm not sure this change is desired.
@hiqsol what's your opinion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They can exclude nic devices with other filter.
If they require comparable searches in two panels obviously they need same filters.
So 2 options:
- convince them that they will not receive comparable searches
- create same filters somehow
Summary by CodeRabbit