fix: refactor getTemplateVar and getTemplateVars methods for improved…#401
fix: refactor getTemplateVar and getTemplateVars methods for improved…#401
Conversation
… clarity and performance
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 618adb3481
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // built-in resource fields | ||
| $result = $this->collectResourceFields($resource, $idnames); | ||
|
|
There was a problem hiding this comment.
$result is now prefilled with built-in resource fields before the TV query runs, which reverses the previous ordering (TV rows first, then built-ins). This changes observable behavior for callers that iterate getTemplateVars('*', ...) and rely on $sort to control the leading records, because entries like alias/id now appear before any sorted TV rows and can alter snippet/output logic that processes the list in order.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
テンプレート変数取得処理（getTemplateVar / getTemplateVars）をリファクタし、リソース取得・組み込みフィールド収集・WHERE句構築をヘルパーメソッドに分離して可読性と保守性を改善する変更。
Changes:
- getTemplateVars内の処理をgetResourceForTV / collectResourceFields / buildTVWhereClauseへ分割
- 組み込みフィールドとユーザー定義TVの取得フローを整理
- $fields引数が後方互換用で未使用である旨のコメントを追加
| } | ||
| if (preg_match('@^[1-9][0-9]*$@', $idnames)) { | ||
| return sprintf('tv.id=%s', $idnames); | ||
| } |
There was a problem hiding this comment.
buildTVWhereClause()が、従来getTemplateVars()で受け付けていた「先頭が'で始まるIN句用の文字列」（例: "'tv1','tv2'"）を扱えなくなっている。旧実装ではこの形式を検出して tv.name IN (...) を構築していたが、現状だとtv.name=''tv1','tv2''のような等価比較になり一致しない。後方互換性のため、先頭が'の場合はIN句として扱う分岐をbuildTVWhereClause()に追加するか、サポート終了を明示して呼び出し側を配列入力へ統一する必要がある。
| } | |
| } | |
| if (is_string($idnames) && strlen($idnames) > 0 && $idnames[0] === "'") { | |
| return sprintf('tv.name IN (%s)', $idnames); | |
| } |
1 participant
This pull request refactors the implementation of template variable retrieval in
document.parser.class.inc.phpto improve code clarity and maintainability. The main changes include extracting logic into new helper methods, clarifying the use of the$fieldsparameter, and simplifying the handling of built-in and user-defined template variables.Refactoring and code clarity improvements:
getResourceForTV,collectResourceFields, andbuildTVWhereClause.getTemplateVarsmethod by replacing inline logic with calls to the new helper methods and clarifying the flow for both built-in and user-defined template variables.Documentation and backward compatibility:
$fieldsparameter is kept for backward compatibility but is no longer used (always selects all columns) in bothgetTemplateVarandgetTemplateVars. [1] [2]… clarity and performance