アルバムの表示順に整合性がないため昇順に変更する

[imamura623]

■現象

アルバムの表示順が投稿日時の降順（投稿日時の新しい順）になっている。アルバムなら表示順は古い順になっているべきのため、デフォルトを投稿日時の昇順（投稿日時の古い順）に変更する。

ただし、アルバム機能は「アルバム」としての用途だけでなく「画像置き場」としての用途も想定しているため、投稿日時の降順（投稿日時の新しい順）に並び替える機能も用意する。

画面案

この変更は、2.14のみで対応する。

■原因

仕様策定時点でアルバム機能を文字通りの「アルバム」しての用途より「画像置き場」としての用途を優先し、新しいものが先にみえるほうが良いと判断したため。

■修正内容

アルバム写真一覧（fh_album_list)の修正点

• デフォルトで昇順（投稿日時の古い順）になるように変更
• 写真一覧の上部右側に降順・昇順（投稿日時の新しい順、古い順）を切り替えるリンクを設置
• PC、携帯ともに修正

日記への写真挿入ダイアログ（h_album_image_insert_dialog）について注意点

写真挿入ダイアログはアルバム写真一覧を呼び出すクラスと同様のクラスを使用しており、本来なら同様の並び順になるはずだが、写真挿入ダイアログについては最新の写真が上段に表示されていた方が利便性が高いと判断し、並び順について降順（投稿日時の新しい順）のまま2.14.1.1と変更が出ないように修正した。

• Replying to imamura623 を参照

■関連情報

​http://sns.openpne.jp/?m=pc&a=page_fh_diary&target_c_diary_id=19862より転記

結局アルバムの表示順ってどうなったんでしょう。
チケット作成→2.13.3で試験実装→アンケートはとった→結論は出さずに放置状態（←イマココ）であってるのかな？

http://sns.openpne.jp/?m=pc&a=page_fh_diary&target_c_diar...
http://sns.openpne.jp/?m=pc&a=page_fh_diary&target_c_diar...

先ほど気が付いたのですが、2.12系で一番左上の写真を開くと、「前の写真」というリンクだけが出てきます。一番右下の写真を開くと、「次の写真」というリンクだけが出てきます。
つまり、個別ページでは「もっとも古い写真が最初でもっとも新しい写真が最後」になる、昇順で並べるという認識の記述です。

一覧ページでは降順なのだから、2.13で*試験*実装するとかいう前に、2.12時点ですでに「昇順」と思って記述している個別ページと「降順」と思って並べている一覧ページが混在して、実は整合性ないんですね。
「時間的に前、次」という解釈も、他の一覧画面や日記画面の「前」「次」と整合しないから、ないと思う。

少なくとも2.12系は「表示バグ」という意味で、enhancementじゃなくてここはdefectを抱えてると思う。実装面では簡単なところで、変に意思決定件を移譲しちゃってグダグダにしてないで、ここはさっさとけりを付けたらいいと思うんだけど。
大体、sns.openpne.jpでアンケートしても、全OpenPNEユーザー（運用者じゃなくて参加者）の1%も参加してないんじゃないかな？ 

[kiwa]

対応まで手が回っていなくて申し訳ありません。既知の問題なので「再現待ち」キーワードを外します。

[kiwa]

画面案

[imamura623]

r12802でコミットしました。コミット内容は以下の通りです。

• デフォルトで画像が日時で昇順表示（PC、ktai）
• 昇順と降順を切り替える機能の追加（PC、ktai）

[imamura623]

r12803でコミットしました。

• webapp/lib/db/album.phpに不要な空白があったので削除

[imamura623]

r12804でtrunkにマージしました。

[imamura623]

webapp/modules/pc/page/fh_album.phpに不要な空白があったので削除しました。

• r12819で2.14xへコミット
• r12820でtrunkへコミット

[imamura623]

kudoさんに動作テストを実施してもらいました。

• デフォルト状態の画像の並び順が逆になってしまっている

差し戻します。

[kiwa]

追加のバグ報告です。

場所

pc_page_fh_album

内容

「表示を降順に変更」リンクを押下した際、URLに「desc=」の値が追加されますが、この値を0,1以外の数値にした場合、アルバム画像が表示されなくなります

補足

この現象が発生した場合、DBエラーが吐き出されます。

Oct 14 19:38:29 db [error] msg:-> DB Error: mismatch     info:->  LIMIT 0, 10 [DB Error: mismatch]

[imamura623]

Replying to imamura623:

kudoさんに動作テストを実施してもらいました。

• デフォルト状態の画像の並び順が逆になってしまっている

差し戻します。

上記についてkiwaさんに確認しましたが、画像の並び順は正しいようです。

validateとUIの修正をr12992 でtrunkに、r12993 で2.14xにコミットしました

[ogawa]

確認中にする前に Description の修正もお願いします。

[imamura623]

r12992 とr12993　の変更時にvalidateを追加しDBエラーは発生しないようになっています。

[ogawa]

以下の点を確認・修正してください。

webapp/modules/pc/templates/fh_album.tpl

• 72-76: 並び順を変更した際には1ページ目に戻るようにした方がよいと思います
• 73,75: desc=({$desc+1}) の部分は単純に desc=1 のように書いた方がよいです
• 74: elseif にする必要はありません。else を使ってください

webapp/modules/ktai/templates/fh_album_image_list.tpl

• webapp/modules/pc/templates/fh_album.tpl と同様

webapp/modules/pc/page/fh_album.php

• 20: 原則としてアクション内で $_GET を使って直接リクエスト変数を取得してはいけません。かわりに $request を使ってください
• 76-78: バリデーションルールで type=bool とした上で $request から取った値であればこの処理は不要です

webapp/modules/ktai/page/fh_album_image_list.php

• webapp/modules/pc/page/fh_album.php と同様

webapp/lib/db/album.php

• db_album_c_album_image_list4c_album_id()
  • 112: Docblock コメントの @param が足りません
  • 117-121: $desc が 0 のときに DESC になり、$desc が 1 のときに ASC になるという内部仕様は混乱を招きます。逆にしてください
  • 117-121: $sql の内容がほぼ重複していますので共通部分は2度書かないようにしてください
  • 119: elseif にする必要はありません。else を使ってください
  • 144: $desc を返り値に含める必要はないと思います
  • pc_page_h_album_image_insert_dialog の並び順にも影響があるようですが、問題ないでしょうか

[imamura623]

Replying to ogawa:

• r13005でtrunkにコミットしました
• r13005でstable-2.14.xにコミットしました

pc_page_h_album_image_insert_dialog の並び順にも影響があるようですが、問題ないでしょうか

この件については、まだ対応していません。対応方法を考えます。

[ogawa]

以下の点を確認・修正してください。また、pc_page_h_album_image_insert_dialog の並び順に関しては対応方法が決まったらコメントと合わせて description を変更しておいてください。

webapp/modules/pc/templates/fh_album.tpl

• 72-76: 1ページ目を指定している部分は省略可能なので、「&page=1」の部分は削除してもよいです
  • シンプルさを重視するなら削除
  • page=1 であることを明示したいのであれば残す
  • どちらでもOKですが、OpenPNE では削除している方が多いと思います
• 72-76: else の部分がこちらの意図が伝わっていなかったようですが、({if !$desc})...({else})...({/if}) のようにするとよりシンプルになってよいと思います

webapp/modules/ktai/templates/fh_album_image_list.tpl

• webapp/modules/pc/templates/fh_album.tpl と同様

webapp/modules/pc/page/fh_album.php

• 77: 空行のスペースを削除してください

webapp/modules/ktai/page/fh_album_image_list.php

• 74: 空行のスペースを削除してください

webapp/lib/db/album.php

• 113: Docblock コメントは今回の修正以前から足りていなかったのが悪いのですが、ついでに $page と $page_size も追加しておいてください
• 113: $desc のように変数名だけではわかりにくい可能性のあるものには説明文を付けるとよりよくなります
  • 例: @param int $desc 並び順を投稿日時の降順にするかどうか(0 or 1)
• 119: if () 内の最後のスペースが不要です
• 119: ここでは if ($desc) { と、条件式を単純化してもOKです

[imamura623]

Replying to ogawa: 修正しました。

• r13013でstable-2.14.xにコミット
• r13012,r13014でtrunkにコミット

また、pc_page_h_album_image_insert_dialog の並び順に関しては対応方法が決まったらコメントと合わせて description を変更しておいてください。

日記への写真挿入は、最近撮った写真を挿入するという動作が多いと思います。この観点からアルバム写真の表示順とは逆になりますが、日記への挿入ダイアログ画面については最新の写真が上段に来るようにしました。

[ogawa]

以下の点を修正してください。

webapp/modules/pc/validate/page/h_album_image_insert_dialog.ini

• 11-13: リクエスト変数から並び順を変更しない部分なのでバリデーションルールには desc は入れないでください

webapp/modules/pc/page/h_album_image_insert_dialog.php

• 20: リクエスト変数から並び順を変更しない部分なので $requests['desc'] は使用しないでください
• 36: $desc の部分は固定で 1 になるようにしてください。$page_size のように事前に 1 を代入しておく形でもよいです

今回の指摘事項はそのままでも実行結果に影響はほとんどなく将来的な拡張性は高まるかもしれませんが、安定版のバグ修正として変更をする以上、コードの変更は必要最小限になるようにした方がよいと思います。

[imamura623]

Replying to ogawa:

修正しました。

• r13017でstable-2.14.xへコミット
• r13018でtrunkへコミット

[ogawa]

確認しました。テスト待ちにします。

[kiwa]

確認しました。問題ありません。