su-its / ams-backend

:briefcase: (This repository is no longer maintained) The backend server of our Access-management-system.
MIT License
0 stars 0 forks source link

ページネーションに必要な情報が足りない、または違う #61

Closed ghost closed 3 years ago

ghost commented 3 years ago

とりあえずページネーションで、暫定的に作ってもらったAPIですが、meta情報に不足があり、代替手段で対応してますが、

仕様変更に弱くなっています。そこでAPIに修正を加えてほしいと思っています。

そんなに大変な作業ではないと思うので、すぐ終わるとは思います。

一応現行の仕様で通そうと思えば暫定的な対応でもなんとかならなくはないです。

変更して欲しい内容は以下になります

access_logsのmeta情報内の

h-takeyeah commented 3 years ago

total_pagetotalper_pageから計算できるからtotalにしてくれという要望ですか? また総件数を表すtotalを追加した後も今までのtotal_pageを残すのはありでしょうか?

h-takeyeah commented 3 years ago

@Stroheim001 念のため確認します.per_pageはdata.lengthと同じ大きさということでよいのですよね.

例えば総件数が25件だった場合1ページ目のper_pageは20,2ページ目のper_pageは5になるということですよね.


ただバックエンド内部でDATA_PER_PAGE(=20)という定数が使用されており,これと若干混ざるなあと思っています.

ghost commented 3 years ago

@h-takeyeah data.lengthが個数と同じという認識であっていればその通りです。 totalは確実に用意しておいてほしいです。 total_pageは放置でも構いません。 per_pageの最後の端数については、buefyのサンプルが不親切すぎて書いてないので分からないのですが、デフォだとどれくらいか、を書いて欲しいだけだと思いますが、最悪書かなくても良いと思います。

h-takeyeah commented 3 years ago

@Stroheim001 buefyのサンプル見ました.2点質問があります.下記は 2602b097f261c2209127cfca7a0c1e00e462970a の動作について検討しています. APIによると確かにper-pageにはそのページの行数を与えてあげれば良さそうですね.端数に関しては確かに謎ですね.

それとは別にcurrentというのもあってこちらはバックエンドから渡されてくるmetaの名称で言うとpageにあたる情報を与えてあげれば良さそうですね.ただ現時点でのフロントエンドの実装には,このcurrentが無いようですが付ける予定はありますか?というのが1点目の質問です.

で,もしつけるなら,バックエンドから渡すmetapagecurrentに名称変更したほうが分かりやすいかと思いますがどうしましょうか?というのが2点目です.

よろしくお願いします.

余談

https://github.com/su-its/ams-frontend/blob/2602b097f261c2209127cfca7a0c1e00e462970a/pages/log.vue#L10-L17

ここで:per-page="10"と指定しているのに描画されるのが20行というのはよく分かりません.まあそれは自分がBuefyとかVueの仕組みが分かってないだけなんで置いておいていいです.

ghost commented 3 years ago

@h-takeyeah

v-modelの存在を勘違いされてるのではないかと思うのですが、そもそもcurrentはバックエンドと一切関係なく独立して動作しています。

個人的には、今誰がどのページを参照しているかなどという情報を保持している必要性を感じません(頓珍漢な発言だったらごめんなさい)

metaの名前を変えるについては可読性の問題ですので、正直「どうでも良い」が感想です。

ドキュメンテーションできちんと保管できるなら多少難しい名称でも十分ですし、逆に簡素な名前でも、ドキュメンテーションが言ってることと違うみたいな悲惨な状態なら、大問題だよ、ってのが感想です。

per-pageはちょっとなんとかならないか聞いてみます。20件はWQHDでもスクロールが必要なレベルなので

h-takeyeah commented 3 years ago

@Stroheim001 もしかしたら誤解されているかもしれないので補足します.

自分が言っているcurrentというのは,フロントエンドのコードに書かれている

<b-pagination
  v-model="current_page"

このcurrent_pageではなくて,Buefy の APIに書かれているこれ

Name Description Type Values Default
current Current page number, use the .sync modifier to make it two-way binding Number 1

のことを指しています.その点を把握したうえでの

v-modelの存在を勘違いされてるのではないかと思うのですが、そもそもcurrentはバックエンドと一切関係なく独立して動作しています。

というコメントだったならすみません.もしそのようには把握していなかったら改めてコメントお願いします.

h-takeyeah commented 3 years ago

多分なんですけどBuefy の APIに書かれているcurrentは単に今このページが全体の何ページ目かという数字を格納するものです.

Googleの検索結果画面で言うと

Google > 1 2 3 次へ

の太字の1がそうです.

h-takeyeah commented 3 years ago

:なんとかとか.なんとかという書き方はvueのバインドであったり何かの関数や変数の省略形の展開のキーワードだということが分かりました.もう少しvueについて分かったら上でコメントした疑問の答えも分かるようになると思います.@Stroheim001 さん説明ありがとうございました.

ghost commented 3 years ago

@h-takeyeah 自分も忘れてることあるので、ちゃんと調べて教えられるようにしておきます。