フロント検索ボックス、商品のタグ名による絞り込み

[matsuoshi]

概要(Overview・Refs Issue)

4.1-feature 向けのPRです。
フロントの検索ボックスにて、タグ名での検索を行えるようにします
#4862
いったんPR作成しましたが、ちょっとクエリ記述部分が悩ましいのでご意見いただけたらと思います。

方針(Policy)

商品に付与されたタグ名を、検索ボックスからの検索対象にします。

例: 「新商品」タグが付与された商品は、「新商品」という文字列で検索した場合、検索にひっかかるようになる

フロントの見た目・UI変更はありません。

実装に関する補足(Appendix)

管理画面側のタグ検索は別PRにて
#4975

テスト（Test)

タグ名による検索、複数のタグ名でのand検索、タグ名と商品名のand検索、のテストを追加しています

相談（Discussion）

現状の検索処理 ProductRepository の getQueryBuilderBySearchData() では、規格の商品コードを検索対象としています。
今回はそれと同様にタグ名を検索対象とするよう、直後にSQLを追記しました。
が、SQLの記述が長くなってきましてこれで良いのか判断しかねています。

たとえばクエリビルダの expr()->orX() あたりを使って分割・書き換えたほうがよいか、
他になにかよい手があればアドバイスいただきたい感じです

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

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

レビュワー確認項目

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

新機能追加に伴う競合確認

• プラグインとの競合がないか → 検索の挙動に問題ありませんでした
  • https://www.ec-cube.net/products/detail.php?product_id=1862
  • https://www.ec-cube.net/products/detail.php?product_id=2157
  • https://www.ec-cube.net/products/detail.php?product_id=1709
  • https://www.ec-cube.net/products/detail.php?product_id=2156
• プラグインからのマイグレーションが必要か

3件タグ関連のプラグインあり、調査してチェック

[tao-s]

tag1, tag2, tag3... とするのではなく、 tag[]として配列でタグを渡す方が冗長なコードが無くなると思うんですが、いかがでしょう？

ProductRepository::getQueryBuilderBySearchData()

        // tag
        $tagJoin = false;
        if (!empty($searchData['tag']) && $searchData['tag']) {
            if ($searchData['tag']) {
                foreach($searchData['tag'] as $tkey => $tag)
                {
                    $qb
                    ->innerJoin('p.ProductTag', 'ptg'.$tkey)
                    ->innerJoin('ptg'.$tkey.'.Tag', 'tg'.$tkey)
                    ->andWhere('ptg'.$tkey.'.Tag = :Tags'.$tkey)
                    ->setParameter('Tags'.$tkey, $tag);
                }
                $tagJoin = true;
            }
        }

Form\Type\SearchProdyctType::buildForm()

        $builder->add('tag', EntityType::class, [
            'class' => 'Eccube\Entity\Tag',
            'choice_label' => 'Name',
            'choices' => $Tags,
            'multiple' => true,
            'expanded' => true,
            'placeholder' => 'common.select__all_products',
            'required' => false,
        ]);

[codecov-commenter]

Codecov Report

Merging #4976 (3d76972) into 4.1 (155be68) will decrease coverage by 0.14%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##                4.1    #4976      +/-   ##
============================================
- Coverage     68.33%   68.18%   -0.15%     
  Complexity     6033     6033              
============================================
  Files           456      456              
  Lines         24929    24929              
============================================
- Hits          17036    16999      -37     
- Misses         7893     7930      +37     

Flag	Coverage Δ
E2E	57.21% <ø> (ø)
Unit	75.61% <ø> (-0.18%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/Eccube/Repository/ProductRepository.php	74.23% <ø> (-22.70%)	⬇️
src/Eccube/Entity/ProductImage.php	66.66% <0.00%> (-6.67%)	⬇️
src/Eccube/Entity/TaxRule.php	86.41% <0.00%> (+2.46%)	⬆️

---

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 155be68...3d76972. Read the comment docs.

[nanasess]

@matsuoshi ユニットテストがすべて落ちているようですので、最新の 4.1 ブランチをマージしてみていただけますでしょうか？
また、 tao さんのコメントご確認お願いします

[tao-s]

@matsuoshi すいません、僕が少し勘違いしていました。
単純にタグを選択して商品を検索するのかと思ってましたが、キーワードでタグを検索するという仕様なんですね。
ただ、この書き方だとキーワードが入力された時のクエリが冗長でパフォーマンスに疑問があります。
ひとつのクエリで全てを処理するのではなく、タグの検索と商品の検索とでクエリを2つに分けるのはどうでしょうか？

[matsuoshi]

@tao-s @nanasess
反応おそくなりすみません、クエリの部分は自分でも不安がありましたのでアドバイス助かります、ありがとうございます。ちょっと検討してみます。

[matsuoshi]

こちら、やはりパフォーマンスの懸念と、外部仕様が適切がどうかという所から再検討したいと思います
(タグを検索したいニーズはあるとして、キーワード欄にてタグ名で検索するという仕様が適切か否か？など)
@tao-s ありがとうございました！

[tao-s]

@matsuoshi ウチの事例だと、キーワードじゃなくて、タグをクリックした際にそのタグで絞り込める様な感じで実装しています。

https://kamiol.com/products/list?tag[]=6&tag[]=8&name=%E3%82%AD%E3%83%A5%E3%83%AA%E3%82%A2