-
Notifications
You must be signed in to change notification settings - Fork 30
GEM: ソート時に補助的なソート列を追加 #1959
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
base: master
Are you sure you want to change the base?
GEM: ソート時に補助的なソート列を追加 #1959
Changes from all commits
7aed099
f7797c0
a2418a3
39249a4
2e48cd8
13667e0
7b9e468
74458fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -221,10 +221,16 @@ public From getFrom() { | |||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||
| public OrderBy getOrderBy() { | ||||||||||||||||||||||||||||||||||||
| OrderBy orderBy = null; | ||||||||||||||||||||||||||||||||||||
| // ソート設定が存在する場合 | ||||||||||||||||||||||||||||||||||||
| // TODO: 以下、分かりづらくないか | ||||||||||||||||||||||||||||||||||||
| // 画面でユーザーが指定したソート列: | ||||||||||||||||||||||||||||||||||||
| // * hasSortSetting() には入ってないが | ||||||||||||||||||||||||||||||||||||
| // * getSortSetting() には入っている | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // TODO: やりたいことは結局:OrderBy = (画面ソート列) + (ソート設定 or OID) | ||||||||||||||||||||||||||||||||||||
| // ∴ 「まず前者を冒頭で処理した後に、後者を場合分けする」方が自然で分かりやすいのでは?(現状は、後者の場合分けから始まっているが) | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
224
to
+231
|
||||||||||||||||||||||||||||||||||||
| // ソート設定が存在する場合 | |
| // TODO: 以下、分かりづらくないか | |
| // 画面でユーザーが指定したソート列: | |
| // * hasSortSetting() には入ってないが | |
| // * getSortSetting() には入っている | |
| // TODO: やりたいことは結局:OrderBy = (画面ソート列) + (ソート設定 or OID) | |
| // ∴ 「まず前者を冒頭で処理した後に、後者を場合分けする」方が自然で分かりやすいのでは?(現状は、後者の場合分けから始まっているが) | |
| // ソート設定・画面ソート指定の扱い: | |
| // ・画面でユーザーが指定したソート列は getSortSetting() には含まれるが、 | |
| // hasSortSetting() の判定対象には含まれない前提となっている。 | |
| // ・本メソッドでは、ソート設定が存在する場合は getSortSetting() の内容 | |
| // (=画面ソート列を含む)をそのまま OrderBy として使用する。 | |
| // ・ソート設定が存在しない場合のみ、後続のデフォルトソート(getSortKey() や OID)を使用する。 | |
| // そのため、結果として OrderBy は | |
| // (画面ソート列を含む getSortSetting の結果) + (ソート設定が無い場合のデフォルトソート) | |
| // という方針で構成される。 |
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[imo] 参照プロパティのソートキー解決ロジックが「ソート設定あり」と「なし」でほぼ重複しており(ここでは TODO でコピペと言及)、将来の仕様変更で片側だけ修正されるリスクがあります。共通メソッド化して1箇所に寄せることを検討してください。
Copilot
AI
Feb 25, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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; |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -31,7 +31,6 @@ | |||||||
| import org.iplass.mtp.entity.query.OrderBy; | ||||||||
| import org.iplass.mtp.entity.query.PreparedQuery; | ||||||||
| import org.iplass.mtp.entity.query.SortSpec; | ||||||||
| import org.iplass.mtp.entity.query.SortSpec.NullOrderingSpec; | ||||||||
| import org.iplass.mtp.entity.query.SortSpec.SortType; | ||||||||
| import org.iplass.mtp.entity.query.Where; | ||||||||
| import org.iplass.mtp.entity.query.condition.Condition; | ||||||||
|
|
@@ -100,47 +99,42 @@ public OrderBy getOrderBy() { | |||||||
|
|
||||||||
| OrderBy orderBy = null; | ||||||||
| if (StringUtil.isNotBlank(sortKey)) { | ||||||||
| // TODO: 【要確認】通常検索(SearchContextBase.getOrderBy())とロジックが違うが、意図的か? | ||||||||
| PropertyDefinition pd = EntityViewUtil.getPropertyDefinition(sortKey, getEntityDefinition()); | ||||||||
| String propName = sortKey; | ||||||||
| if (pd instanceof ReferenceProperty) { | ||||||||
| propName = sortKey + "." + Entity.OID; | ||||||||
| } | ||||||||
| if (StringUtil.isBlank(sortType)) { | ||||||||
| orderBy = new OrderBy().add(new SortSpec(propName, SortType.DESC)); | ||||||||
| } else { | ||||||||
| orderBy = new OrderBy().add(new SortSpec(propName, SortType.valueOf(sortType))); | ||||||||
| } | ||||||||
| } else if (filter != null && StringUtil.isNotBlank(filter.getSort())) { | ||||||||
| String propName = pd instanceof ReferenceProperty ? sortKey + "." + Entity.OID : sortKey ; | ||||||||
| orderBy = new OrderBy().add(new SortSpec(propName, StringUtil.isBlank(sortType) ? SortType.DESC : SortType.valueOf(sortType))); | ||||||||
| } | ||||||||
| //TODO: addする際に重複チェックするべきか? | ||||||||
|
||||||||
| //TODO: addする際に重複チェックするべきか? | |
| // NOTE: 画面指定ソートとフィルタ/設定ソートで同一キーが指定された場合も、そのまま ORDER BY に連結し、 | |
| // 重複するソートキーをあえて削除しない仕様とする(DB 上は冗長だが挙動には影響しないため)。 |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -101,7 +101,7 @@ public enum CsvUploadTransactionType { | |||||
| /** 詳細条件の表示件数 */ | ||||||
| @MetaFieldInfo( | ||||||
| displayName="詳細条件の表示件数", | ||||||
| displayNameKey="generic_element_section_SearchConditionSection_conditionDispCountDisplayNameKey", | ||||||
| displayNameKey="generic_element_section_SearchConditionSection_conditionDispCountDisplayNameKey", //TODO: locale_ja.jsにキー追加 | ||||||
|
||||||
| displayNameKey="generic_element_section_SearchConditionSection_conditionDispCountDisplayNameKey", //TODO: locale_ja.jsにキー追加 | |
| displayNameKey="generic_element_section_SearchConditionSection_conditionDispCountDisplayNameKey", // NOTE: 本キーのロケール定義はクライアント側リソース(本モジュール外)で管理する前提 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[ask]
getOrderBy()末尾の TODO にある通り、このクラスで SearchLayout 側のソート条件(hasSortSetting()/getSortSetting())を無視するのが仕様として正しいか判断が必要に見えます。FixedSearch のUX/設定項目として SearchLayout のソートが期待されるなら取り込む、期待されないなら TODO を消して「無視する理由」をコメント等で明確化してください。