Skip to content
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

セッション概要が、一覧ページと詳細ページで不一致なケースがある #450

Open
taisukeoe opened this issue Sep 17, 2020 · 16 comments
Labels
bug Something isn't working

Comments

@taisukeoe
Copy link
Contributor

Scala Beginner向けセッションについて、
採用後に概要を書き足してもらったとき、acceptedのみしか更新しなかったことが理由と思われます。

#448 をレビューしていて気づきました。

@taisukeoe taisukeoe added the bug Something isn't working label Sep 17, 2020
@sugoikondo
Copy link
Contributor

これは不一致な部分だけを直せばよいのか、それとも参照先を統一するべきなのかで言うと、どちらが良いですかね?

統一した場合、本セッションとプロポーザルとで違うタイトル・説明にしたいといったケースがあると難しい気もしますが、そもそもそういったことが有るのか知りたいところです

@taisukeoe
Copy link
Contributor Author

それでいうと、参照先を統一する(どちらもacceptedを参照するようにする)のがベストな気がします~!

@taisukeoe
Copy link
Contributor Author

それでいうと、参照先を統一する(どちらもacceptedを参照するようにする)のがベストな気がします~!

44ad4c6 で対応してみました

@sugoikondo
Copy link
Contributor

あ、でいうと今回ボクが対応した方のは参照先が該当ストアではないので、プログラム一覧のところには反映されないですね。。

@taisukeoe
Copy link
Contributor Author

taisukeoe commented Oct 5, 2020

@sugoikondo
お、どれを指して該当ストアとおっしゃってるか分からなかったですが、タイムテーブルに関連するのってsessions.ts(タイムテーブルとそのモーダルの表示)と、 proposals.ts (セッションクリック後の詳細ページ) の2つだという認識で、今回その両方とも改修しました。

なにか認識違いありそうですか?

@sugoikondo
Copy link
Contributor

あ、なるほど。。こうなったわけですね。。。。

勘違いでした。

import proposalData from '~/data/top/acceptedSessions.json'

@sugoikondo
Copy link
Contributor

でもこれ、結局参照先の統一ではなく、変えただけではないかと思うのですが、これで良い感じですか?

@taisukeoe
Copy link
Contributor Author

どっちもacceptedSessionsを参照してるので、統一されてるような気がしましたが、近藤さんが統一されてないと仰ってるのはどのへんの話ですか?

(例外としてfilterProposalsByLengthAndLangはプロポーザル一覧画面の表示用なので、proposals/all.jsonからfilterしないといけない、という意味では統一されていませんが。。。)

@sugoikondo
Copy link
Contributor

あ〜、(どちらもacceptedを参照するようにする) なのか、大変失礼いたしました。

@taisukeoe
Copy link
Contributor Author

taisukeoe commented Oct 5, 2020

お、なにか齟齬がとけたようで良かったです。なにか僕の説明がわかりにくかったとしたらごめんなさい。

@taisukeoe
Copy link
Contributor Author

(どこかでproposalsやsessionsといった用語の整理はしたい気持ちはあります。今回ちょっと混乱させてしまうような改修をしてしまったので)

@sugoikondo
Copy link
Contributor

私の理解は、そもそもセッション情報がproposalとacceptedSessionsに分かれているのを統一して、acceptedSessionsにはProposal.Id てきなものだけを残す認識でした。

なのでacceptedSessionsは、idの配列だけになるくらいの認識でした。

@sugoikondo
Copy link
Contributor

どこかでproposalsやsessionsといった用語の整理はしたい気持ちはあります。今回ちょっと混乱させてしまうような改修をしてしまったので

これは僕が作ったTypeScript のモデルにも言えそうです。。

@taisukeoe
Copy link
Contributor Author

なのでacceptedSessionsは、idの配列だけになるくらいの認識でした。

あーなるほどです。もちろん本当はその方がいいですね。。。。

@taisukeoe
Copy link
Contributor Author

TODOとして書き残しておきます

@taisukeoe
Copy link
Contributor Author

余談ですが、 @sugoikondo さんがTypeScript対応してくれたおかげで、proposals.tsとsessions.ts、どちらもめっちゃ触りやすかったです。ありがとうございました! 👍 👍 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants