getTemplateVars()で$fieldsを指定するとエラーになる

[okazunori]

ほぼ影響はないと思いますがご報告します。
getTemplateVars()で$fieldsに空文字と'*'以外の文字列を指定するとArray to string conversionエラーになります。

コード: 全て選択

# document.parser.class.inc.php Line:4754から
        if ($fields === '*' || $fields === '') {
            $fields = "tv.*, IF(tvc.value!='',tvc.value,tv.default_text) as value";
        } else {
            $fields = sprintf(
                "%s, IF(tvc.value!='',tvc.value,tv.default_text) as value",
                array_map(
                    function ($v) {
                        return 'tv.' . $v;
                    },
                    explode(',', $fields)
                )
            );
        }

sprintf()の%sに対して、array_map()の戻り値である配列を渡しているのが原因だと思います。
evo()->join()を使用して、

コード: 全て選択

        if ($fields === '*' || $fields === '') {
            $fields = "tv.*, IF(tvc.value!='',tvc.value,tv.default_text) as value";
        } else {
            $fields = sprintf(
                "%s, IF(tvc.value!='',tvc.value,tv.default_text) as value",
                evo()->join(',', explode(',', $fields), 'tv.')
            );
        }

とすれば回避できました。

'value'は指定しなくても取得しますが、これは仕様なんですね。
と思ったのですが、$fields='value'とするとUnknown column 'tv.value' in 'field list'エラーになります。
実装した方には大変申し訳ないのですが、修正する手間と現実には'*'しか使われていないことを考えると、いっそのこと

コード: 全て選択

$fields = "tv.*, IF(tvc.value!='',tvc.value,tv.default_text) as value";

だけにして['id']や['value']で取得させる方が良い気がしてきました。

[okazunori]

下手くそですが修正案です。
$fields='value'、$fields='id, value'でも取得できます。

コード: 全て選択

        if ($fields === '*' || $fields === '') {
            $fields = "tv.*, IF(tvc.value!='',tvc.value,tv.default_text) as value";
        } else {
            $fieldsArr = array_map('trim', explode(',', $fields));
            $fieldsArr = array_diff($fieldsArr, ['value']);
            $joinedFields = evo()->join(',', $fieldsArr, 'tv.');
            $fields = ($joinedFields ? $joinedFields . ',' : '')
            . "IF(tvc.value!='',tvc.value,tv.default_text) as value";
        }

[yama]

https://github.com/modxcms-jp/evolution ... -t2032.zip
修正とリファクタリングを行いました。こちらで確認いただいてよいでしょうか？
$fieldsは決め打ちで「*」と見なすことにしました。これで困るケースはないと思います。。

https://github.com/modxcms-jp/evolution-jp/pull/401
こちらにプルリクとしてまとめています。

[okazunori]

修正ありがとうございます！
$fieldsに何を指定しても'*'になるということですね。
$fieldsを使っていた人はいなかったと考えられるので、これでいいと思います

ついでに$idnamesについて提案してよろしいでしょうか。
$idnames='*'とするとテンプレート変数だけでなくリソース変数も全取得します。
形式も[[x] => ['name' => $key, 'value' => $value]]とわざわざ二次元配列に変換しています。
これはgetTemplateVarsという名前からすると少しとまどう動作のような気がします。
リソース変数取得はgetDocumentsにおまかせして、本関数はテンプレート変数取得に特化する、というのはどうでしょうか？

といいますのも、この仕様の影響でgetTemplateVarOutputで$idnames='*'が（デフォルトにも関わらず）使えません。
getTemplateVarOutput内でgetTemplateVars('*', ...)として呼び出してしまうのが原因です。
リソース変数はキーが'name'と'value'しかないため"undefined KEY"エラーになってしまいます。

$fieldsの件はなんとなく見つけた不具合でしたが、こちらは少し困っています。
とりあえずgetTemplateVarOutputに下記の一文を追加して対応しています。

コード: 全て選択

    $output = [];
    foreach ($result as $row) {
        if (!isset($row['id'])) { continue; } // <--- 追加。テンプレート変数にはIDがある。
        if (!empty($this->previewObject[$row['name']]) && $docid == $this->documentIdentifier) {

すごーくニッチな提案なので優先度は最低で良いですが、if (!isset($row['id'])) { continue; }だけは採用していただきたいです！

---

ここからは蛇足です。
getTemplateVarsに戻りますが、'*'以外の指定も微妙です。
例えば、配列['templatVar1', 'templatVar2', 'resourceVar']を指定するとテンプレート変数'templatVar1'、 'templatVar2'のみ取得します。
カンマ区切りの文字列'templatVar1,templatVar2,resourceVar'だとリソース変数'resourceVar'のみ取得します。

１）collectResourceFields内
if ($idnames === '*' || (is_string($idnames) && in_array($key, explode(',', $idnames))))
で配列だとfalseになり、文字列だとtrueになる。その結果リソース変数のみ取得。
２）buildTVWhereClause内でWHERE句が
配列だと「tv.name IN ('templatVar1','templatVar2','resourceVar')」でテンプレート変数取得。
文字列だと「tv.name='templatVar1,templatVar2,resourceVar'」になってしまい、何も取得できない。

こういうことだと思います。
配列で指定すれば私の希望っぽい戻り値になるのが面白いです
長文失礼しました。