Conversation
Fix #59 QiitaOrganizationのYassLabのページから投稿数といいねをスクレイピングして いたが，この部分のQiita側の構成が異なっていた． そのためQiitaの現在のページに合わせたCSSセレクタを取得するように修正した
QiitaへのURLを使いまわしているので定数化した
|
ローカルから無事取得できている事を確認した
|
|
現在のコードだと「 |
|
あ，しかしそもそも取得できない時はQiita側に何らかの問題が発生している(もしくは何かしらのCSSの変更があり，スクレイピング部分を修正する必要がある)ので，rescueで例外として扱って上げたほうが適切かもしれない... |
|
お！早速の対応ありがとうございます! 😆 まずは Hotfix をリリースしないと、この修正をしている間も Web サイト https://yasslab.jp/ja ではエラーが表示され続けてしまうので、当該コミットを git cherry-pick して master にマージしておきますね ;) 🚀 ✨ |
|
上記の Hotfix リリースに合わせて、次のコミットで spec を一旦コメントアウトしたので、この PR で作業が終わったらコメントアウトを解除してもらえれば ;) (僕の方で Description に書き足しておきました! ✅ ) |
|
Hotfix release done...!! 🚀✨✅
|
|
@yasulab おっデプロイとspec側の処理ありがとうございます!! 🙏 |
|
d(￣ ￣)✨ |
|
メソッドチェーンでシンプルに実装していたが，qiita側のエラーで値が帰ってこない場合に死ぬという事が解ったので，一旦変数にキャッシュした． 次に問題になってくるのはQiita側が404で落ちた場合，jekyllが起動時に例外を吐いて死ぬ為，この場合も中でよしなにcatchし0を返す設計に変更する |
|
Jekyll側でエラーハンドリング出来るかな...と思い調べていたが見つけきれなかったので，純粋にrubyのエラーハンドリング方法に乗っていくのが良さそう |
|
rubyの例外処理ではrescueを使うとあったのでとりあえず，mechanize関連で例外が発生した際に 試しにアクセスするURLを
|
現在のQiitaから情報を取得するrspecはモックを利用しているが，このhtmlの内 容を現在のQiitaの物に修正した． また値がなかった場合のエラーモックは，Qiitaの投稿数とイイネの数の要素を 消去したhtmlを用意した
Qiitaから情報を取得する箇所のテストでは，htmlに書かれている数をマッチす るか確認しているが，この確認する数を現在のモックに合わせて修正した
今まで変数でやり取りしていたので定数化した
cf. #61 (comment) テストの実装まで終わったので戻した
|
当初の予定のテスト及び，Qiita側で障害が発生し値が取れなかった場合にとりあえず0を挿入し, yasslab.jp側を落とさない設計で実装し直してみました．お手すきの時にレビューお願いします 🙏 @yasulab レビュー後の動きとしてはsquashして1 commitに纏めようと思っています! |
@AnaTofuZ 値が取れなかった場合のデフォルト値についてですが、「０」ではなくて現時点での最新値にするのはどうでしょう? 🤔 「０」と表示されると明らかに異常な値だとエンドユーザー（訪問者）も気づきますが、こうすれば何らかの理由で取得できかった場合も、エンドユーザー側にある程度の情報（最新の値ではないかもしれないけど会社の規模感としては正しい情報）を伝えられるのではないかなと思いました 💭 取得できなかった場合のデフォルト値
|
_plugins/qiita.rb
Outdated
| if element.respond_to? :children | ||
| @items = element.children.text.strip.to_i | ||
| else | ||
| @items = 0 |
There was a problem hiding this comment.
@items の行の 0 と、下にある @likes の行の 0 をいじればいいだけかな🤔💭 僕の方でコミット積んでマージしちゃうのが早そう ;)
|
@yasulab レビューありがとうございます! (今確認しました 🙏 ) テストの方も治す必要があるので作業します! |
|
あ、spec も直さないとかな 🤔💭 できれば spec の定数も |
|
ありがとうございます:pray: 僕は定数使ってなかったので参考になります! テスト側の修正を行ってなかったので, 一旦テスト側を少し修正してまたレビューをお願いしようと思います． |
|
specの定数を使いまわしたいので，今使っているmockのhtmlの中にerbあたりの形式で定数を動的に埋め込んでテストする方向が良さそう..? 🤔 |
| FAILED_MOCK_PATH = 'spec/error_mock.html'.freeze | ||
|
|
||
| RSpec.describe 'Qiita' do | ||
| require './_plugins/qiita.rb' |
There was a problem hiding this comment.
なるほどここで require しているということは _plugins/qiita.rb で宣言している次の定数も使いまわせそう 👀
require 'mechanize'
module Jekyll
QIITA_ORGANIZATION_URL = 'https://qiita.com/organizations/yasslab'
QIITA_PRESET_ITEMS = 192
QIITA_PRESET_LIKES = 6941
...
了解です! 😆👌 Preset などの値はコードの途中にあると見落としたり忘れたりしやすいので、僕が書くときはいつも冒頭に (より具体的にはファイルを開いたときにすぐに気付きやすい位置に) 書くことが多いですね 👀 💭 |
ですね ;) せっかくなので JS 側の方でも同じような修正を入れておこうと思います 🔧 💨 (が、この PR とはコンテキストが異なるので別 branch でやっちゃいますね) |
|
JS 側の対応 Done ✅
|
|
良さそうな雰囲気を感じる ;) 🙆 💖 |
|
mockの要素を動的に変更しようかな〜などと思っていましたが，よくよく考えればHTMLから値が抜き出せている事が確認できれば問題ないので，一旦これで完了かなと思いました． |
|
良さそう! 😆👌✨ 一旦マージして、もし後日何か改善点を見つけたら僕の方で対応しておきますね ;) 🔧💨 PR Thx...!! && 良いお年を〜 🎉 💖 |
|
ありがとうございます:pray: yasulabさんも良いお年を~｡(๑•̀ㅂ•́)و✧ |
背景
#59
原因に関して
以前のwebページリニューアルの際にQiitaOrganizationからデータを持ってくる方法はJekyll側でスクレイピングをするという手段だった．
cf. #8
このPR時にはQiitaのページが変わることが無いという前提の元だったが，最近のリニューアルで大幅に構成が変わり，正常にスクレイピングが出来てなかった．
cf. https://blog.qiita.com/improve-organization-pages/
やること
スクレイピングするCSSセレクタを修正する
テストで利用しているモックのhtmlを現在のQiitaの仕様に修正する
テストを修正する
(取得できない場合の挙動を実装する) ( 実施するならテストを書く)
コメントアウトしている CI の spec/qiita.rb を解除する
テストの設計
テストで使用しているmockの中の定数を動的に使用出来るようにする