Conversation
There was a problem hiding this comment.
Pull request overview
GEM の検索結果ソートにおいて、ユーザーが画面上で指定したソート条件だけでは順序が一意にならないケースを改善し、ページング等の安定性を上げるために「補助的なソート列」を末尾に追加できるようにするPRです(Issue #1795 対応)。
Changes:
- EntityView 系(
SearchContextBase)で、画面ソート時に補助キーとしてOIDを末尾に追加 - TopView の検索結果パーツ(
SearchListContext)で、画面ソート時に Admin 側のソート設定 orupdateDateを末尾に追加 - 固定検索(
FixedSearchContext)でも、画面ソート+フィルタ定義ソートの合成と補助ソート追加の流れを整理
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| iplass-gem/src/main/java/org/iplass/mtp/view/generic/element/section/SearchConditionSection.java | displayNameKey 行に TODO コメントを追加 |
| iplass-gem/src/main/java/org/iplass/gem/command/generic/search/SearchListContext.java | 画面ソートにフィルタ/設定ソートや updateDate を追加できるよう OrderBy 組み立てを変更 |
| iplass-gem/src/main/java/org/iplass/gem/command/generic/search/SearchContextBase.java | 画面ソート時の順序一意化のため OID を補助キーとして追加 |
| iplass-gem/src/main/java/org/iplass/gem/command/generic/search/FixedSearchContext.java | 画面ソート→フィルタ定義ソート→補助ソート、の順で OrderBy を構築するよう変更 |
| // 参照プロパティの場合、画面上の表示項目でソート | ||
| // TODO: 上のロジックのコピペ。解消できないか | ||
| if (pd instanceof ReferenceProperty) { | ||
| if (property.getPropertyName().equals(sortKey)) { |
There was a problem hiding this comment.
[imo] 参照プロパティのソートキー解決ロジックが「ソート設定あり」と「なし」でほぼ重複しており(ここでは TODO でコピペと言及)、将来の仕様変更で片側だけ修正されるリスクがあります。共通メソッド化して1箇所に寄せることを検討してください。
| @MetaFieldInfo( | ||
| displayName="詳細条件の表示件数", | ||
| displayNameKey="generic_element_section_SearchConditionSection_conditionDispCountDisplayNameKey", | ||
| displayNameKey="generic_element_section_SearchConditionSection_conditionDispCountDisplayNameKey", //TODO: locale_ja.jsにキー追加 |
There was a problem hiding this comment.
[imo] displayNameKey 行に TODO コメントが追加されていますが、このリポジトリ内には locale_ja.js が見当たらず(少なくとも iplass-gem 配下には存在しません)、どこにキーを追加すべきかが不明確です。マージ前にキー定義の配置先を明確化して実際に追加するか、参照先が別リポジトリ/別モジュールならその前提をコメントで補足してください。
| displayNameKey="generic_element_section_SearchConditionSection_conditionDispCountDisplayNameKey", //TODO: locale_ja.jsにキー追加 | |
| displayNameKey="generic_element_section_SearchConditionSection_conditionDispCountDisplayNameKey", // NOTE: 本キーのロケール定義はクライアント側リソース(本モジュール外)で管理する前提 |
| return orderBy; | ||
| NullOrderingSpec nullOrderingSpec = getNullOrderingSpec(property.getNullOrderType()); | ||
| // ソート順序を一意にするため、OIDをソートキーの末尾に追加 | ||
| return new OrderBy().add(sortKey, getSortType(), nullOrderingSpec).add(Entity.OID, SortType.ASC); |
There was a problem hiding this comment.
[must] sortKey が参照プロパティのネスト未設定等で Entity.OID にフォールバックした場合でも、末尾でさらに Entity.OID を add しているため、ORDER BY OID <dir>, OID ASC のように同一キーが重複します(方向が異なる可能性もあり、意図が不明瞭です)。sortKey が OID に確定したケースでは補助キー追加をスキップする等、OID の二重追加を避けてください。
| return new OrderBy().add(sortKey, getSortType(), nullOrderingSpec).add(Entity.OID, SortType.ASC); | |
| boolean isOidSortKey = Entity.OID.equals(sortKey); | |
| OrderBy orderBy = new OrderBy().add(sortKey, getSortType(), nullOrderingSpec); | |
| if (!isOidSortKey) { | |
| orderBy.add(Entity.OID, SortType.ASC); | |
| } | |
| return orderBy; |
4bc7549 to
9400c3d
Compare
要確認ドキュメントへの記載も必要か |
9400c3d to
74458fd
Compare
74458fd to
5ce9f1a
Compare
5ce9f1a to
5828b57
Compare
631a6e5 to
a9f16bc
Compare
007700d to
5eeadf5
Compare
fafc7fc to
e2d39f9
Compare
対応内容
GEMの検索時に
ソート時に補助的なソート列を追加(∵ 結果が一意になるように)
closes #1795
動作確認・スクリーンショット(任意)
レビュー観点・補足情報(任意)