Skip to content
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

[bugfix]: issue6 - modify index to uniques for HasOne relation #10

Draft
wants to merge 1 commit into
base: 1.x
Choose a base branch
from

Conversation

EvgenyBarinov
Copy link
Contributor

spiral/cycle-bridge#50
Checked on local project:
Scenario to check it:

  1. Create User entity
/**
 * @Cycle\Entity(
 *     database="core",
 *     table = "users",
 *     repository="App\Repository\Core\UserRepository"
 * )
 * @Cycle\Table(
 *     indexes={
 *          @Cycle\Table\Index(columns={"email"}, unique=true)
 *     }
 * )
 */
class User extends AbstractEntity
{
    /**
     * @Cycle\Relation\HasOne(
     *     target="UserProfile",
     *     outerKey="user_id"
     * )
     */
    public UserProfile $userProfile;

    /**
     * @Cycle\Column(type = "string(180)", name = "email")
     */
    public string $email;

    /**
     * @Cycle\Column(type = "bigPrimary", name = "id")
     */
    protected ?int $id = null;
}
  1. Create UserProfile entity
/**
 * @Cycle\Entity(
 *     database="core",
 *     table = "user_profile",
 *     repository="App\Repository\Core\UserProfileRepository"
 * )
 */
class UserProfile extends AbstractEntity
{
    /**
     * @Cycle\Relation\BelongsTo(
     *     target="User",
     *     innerKey="user_id"
     * )
     */
    private User $user;

    /**
     * @Cycle\Column(type = "bigPrimary", name = "id")
     */
    protected ?int $id = null;
}
  1. Run command cycle:migrate

    3.1. Check new migration up method:

public function up()
{
    $this->table('users')
        ->addColumn('email', 'string', [
            'nullable' => false,
            'default'  => null,
            'size'     => 180
        ])
        ->addColumn('id', 'bigPrimary', [
            'nullable' => false,
            'default'  => null
        ])
        ->addIndex(["email"], [
            'name'   => 'users_index_email_5eda12d26660e',
            'unique' => true
        ])
        ->setPrimaryKeys(["id"])
        ->create();
    
    $this->table('user_profile')
        ->addColumn('id', 'bigPrimary', [
            'nullable' => false,
            'default'  => null
        ])
        ->addColumn('user_id', 'bigInteger', [
            'nullable' => false,
            'default'  => null
        ])
        ->addIndex(["user_id"], [
            'name'   => 'user_profile_index_user_id_5eda12d264da5',
            'unique' => true
        ])
        ->addForeignKey(["user_id"], 'users', ["id"], [
            'name'   => 'user_profile_foreign_user_id_5eda12d264dfd',
            'delete' => 'CASCADE',
            'update' => 'CASCADE'
        ])
        ->setPrimaryKeys(["id"])
        ->create();
}

@codecov
Copy link

codecov bot commented Jun 5, 2020

Codecov Report

Merging #10 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      spiral/framework#10   +/-   ##
=========================================
  Coverage     89.30%   89.30%           
  Complexity      397      397           
=========================================
  Files            31       31           
  Lines          1000     1000           
=========================================
  Hits            893      893           
  Misses          107      107           
Impacted Files Coverage Δ Complexity Δ
src/Relation/HasOne.php 100.00% <100.00%> (ø) 9.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9933179...12990f8. Read the comment docs.

@wolfy-j
Copy link
Contributor

wolfy-j commented Jun 8, 2020

It must be optional ( need an additional key ), otherwise, it will affect existing schemes.

@EvgenyBarinov EvgenyBarinov marked this pull request as draft June 8, 2020 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

2 participants