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

ログインユーザー名をカスタマイズできるよう変更 #4687

Closed
wants to merge 4 commits into from

Conversation

izayoi256
Copy link
Contributor

概要(Overview・Refs Issue)

#4617

以下、ログインユーザー名を電話番号に変更するカスタマイズ例。

<?php

namespace Customize\Entity;

use Eccube\Annotation\EntityExtension;

/**
 * @EntityExtension("Eccube\Entity\Customer")
 */
trait CustomerTrait
{
    private static $usernameField = 'phone_number';
}

方針(Policy)

property_existsを使用するLaravelっぽい実装。

テスト(Test)

未実装。

相談(Discussion)

  • Entity Proxyを前提とした機能に対するテストの書き方が不明。
  • 現状では静的変数で定義する形だが、静的関数じゃないと対応できないケースが存在するかどうか。(下記に例示)
<?php

namespace Customize\Entity;

use Eccube\Annotation\EntityExtension;

/**
 * @EntityExtension("Eccube\Entity\Customer")
 */
trait CustomerTrait
{
    private static function usernameField(): string
    {
        if ('なんらかの条件分岐や') {
            return 'email';
        }
        return 関数呼び出しなど();
    }
}

マイナーバージョン互換性保持のための制限事項チェックリスト

  • 既存機能の仕様変更
  • フックポイントの呼び出しタイミングの変更
  • フックポイントのパラメータの削除・データ型の変更
  • twigファイルに渡しているパラメータの削除・データ型の変更
  • Serviceクラスの公開関数の、引数の削除・データ型の変更
  • 入出力ファイル(CSVなど)のフォーマット変更

レビュワー確認項目

  • 動作確認
  • コードレビュー
  • E2E/Unit テスト確認(テストの追加・変更が必要かどうか)
  • 互換性が保持されているか
  • セキュリティ上の問題がないか

@okazy okazy added the enhancement 機能追加 label Sep 24, 2020
@okazy okazy added this to the 4.0.x milestone Sep 24, 2020
@okazy
Copy link
Contributor

okazy commented Sep 24, 2020

スマートで必要十分な拡張方法だと思いました。
まだ全部見切れてないのですが、忘れないように気づいた点をメモしておきます。

  • UnitTestだけなら ReflectionClass でクラス変数を足してやればいいかと思って調べてみたけどクラス変数の追加はできなさそう。
  • ちゃんとやるならE2Eテストでプラグインのインストールをするのが良いか。
  • 有効なtraitがなくなった場合にproxyを更新できない?(本件の変更とは関係なし)

@okazy
Copy link
Contributor

okazy commented Sep 28, 2020

動作確認をして問題ありあませんでした。
テストの実装方法はわからないままです。。。

Entity拡張自体の課題が見つかったので別途Issue化しました。 #4708

@kiy0taka
Copy link
Contributor

拡張機能としてユーザ名を別のものにするためには、変更するプロパティをユニークにする仕組みもセットで必要になるかと思います。
このPRのコード例では、phone_number がユニークにできればいいかと思います。

@izayoi256
Copy link
Contributor Author

それはカスタマイズ側の責任でいいのではないでしょうか?
例示したコードはあくまで例なので、想定されるユースケースでは例えばユニークなlogin_idみたいなカラムを追加すると思いますし…。

@kiy0taka
Copy link
Contributor

確かにユースケースとしては拡張したプロパティでの利用になりそうですね。
ユニークにするのはカスタマイズでとしましょう。

@okazy
Copy link
Contributor

okazy commented Nov 6, 2020

4.1ではSerializableの実装をしており、影響がありそうなのでメモとしてリンクしておきます。
#2874
0824c7e
9a84daf

@okazy
Copy link
Contributor

okazy commented Apr 1, 2021

EC-CUBE 4.1 ではシリアライズの処理を実装しましたので、そちらも考慮もする必要があります

/**
* String representation of object
*
* @see http://php.net/manual/en/serializable.serialize.php
*
* @return string the string representation of the object or null
*
* @since 5.1.0
*/
public function serialize()
{
// see https://symfony.com/doc/2.7/security/entity_provider.html#create-your-user-entity
// MemberRepository::loadUserByUsername() で Work をチェックしているため、ここでは不要
return serialize([
$this->id,
$this->login_id,
$this->password,
$this->salt,
]);
}
/**
* Constructs the object
*
* @see http://php.net/manual/en/serializable.unserialize.php
*
* @param string $serialized <p>
* The string representation of the object.
* </p>
*
* @return void
*
* @since 5.1.0
*/
public function unserialize($serialized)
{
list(
$this->id,
$this->login_id,
$this->password,
$this->salt) = unserialize($serialized);
}

@izayoi256
Copy link
Contributor Author

@okazy
対応してpushしました。 コンフリクトは4.1-coreを取り込んだことによるもののようですが…。

@izayoi256 izayoi256 changed the base branch from 4.0 to 4.1-core April 2, 2021 01:47
@izayoi256 izayoi256 changed the base branch from 4.1-core to 4.1-feature April 2, 2021 01:48
@izayoi256 izayoi256 changed the base branch from 4.1-feature to 4.1-core April 2, 2021 01:48
@izayoi256
Copy link
Contributor Author

PR先を4.1-coreに変更しました。

@chihiro-adachi chihiro-adachi changed the base branch from 4.1-core to 4.1 September 5, 2021 23:45
@chihiro-adachi chihiro-adachi modified the milestones: 4.0.x, Not release Jun 2, 2022
@chihiro-adachi
Copy link
Contributor

@izayoi256
こちらありがとうございました。
decoratesでCustomer/MemeberProvider拡張してもらえればそれで十分実装できそうです。
ご対応いただいたのにすみません。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 機能追加
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants