Skip to content

Conversation

@niqingyang
Copy link
Contributor

…erface

Q A
Is bugfix?
New feature? ✔️
Breaks BC?
Fixed issues comma-separated list of tickets # fixed by the PR, if any
/**
 * User model
 * 
 * @property int $id
 * @property string|null $name
 * @property AuthItem[] $authItems
 */
class User extends ActiveRecord {
  public function tableName() {
    return '{{%admin}}';
  }

  public function getAuthItemsQuery() {
     return $this->hasMany(AuthItem::class, ['name' => 'item_name'])->viaTable('{{%auth_assignment}}', ['user_id' => 'id']);
  }

  public function getRelationQuery(string $name): ActiveQueryInterface {
    if ($name === 'auth_items') {
      $name = 'authItems';
    }
    return parent::getRelationQuery($name);
  }

  public function hasRelationQuery(string $name): bool {
    return $name === 'auth_items' || parent::hasRelationQuery($name);
  }
}

$user = new User();
$user->authItems; // change before ✔️
$user->auth_items; // support change aftrer ✔️

@codecov
Copy link

codecov bot commented Jul 16, 2025

Codecov Report

Attention: Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 89.20%. Comparing base (de8fb61) to head (c721f10).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/Trait/MagicPropertiesTrait.php 83.33% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #436      +/-   ##
============================================
- Coverage     89.40%   89.20%   -0.21%     
- Complexity      575      576       +1     
============================================
  Files            17       17              
  Lines          1387     1389       +2     
============================================
- Hits           1240     1239       -1     
- Misses          147      150       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@samdark samdark requested review from Tigrov and Copilot and removed request for Tigrov July 16, 2025 09:21
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a new method hasRelationQuery(string $name): bool to the ActiveRecordInterface and its implementation to check if a relation query method exists for a given name. This enhances the magic property handling by replacing direct method_exists calls with a more explicit and overridable method.

  • Adds hasRelationQuery() method to interface and abstract implementation
  • Updates magic property getter to use the new method instead of direct method_exists check
  • Includes comprehensive test coverage for the new functionality

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/ActiveRecordInterface.php Adds the interface declaration for hasRelationQuery() method
src/AbstractActiveRecord.php Implements the hasRelationQuery() method with default behavior
src/Trait/MagicPropertiesTrait.php Updates magic property getter to use new method and adds PHPDoc annotation
tests/ActiveRecordTest.php Adds comprehensive test coverage for the new method functionality

}

if (method_exists($this, "get{$name}Query")) {
if ($this->hasRelationQuery($name)) {
Copy link
Member

Choose a reason for hiding this comment

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

Worth checking performance after this change.

@Tigrov
Copy link
Member

Tigrov commented Jul 21, 2025

@niqingyang niqingyang closed this Jul 23, 2025
@niqingyang niqingyang reopened this Jul 23, 2025
@Tigrov Tigrov merged commit 7b74a7b into yiisoft:master Jul 23, 2025
46 of 71 checks passed
@Tigrov
Copy link
Member

Tigrov commented Jul 23, 2025

👍

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.

4 participants