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

Design getting urls #12

Merged
merged 28 commits into from
Jul 14, 2014
Merged

Design getting urls #12

merged 28 commits into from
Jul 14, 2014

Conversation

daicche
Copy link
Contributor

@daicche daicche commented Jun 24, 2014

概要

検索結果ページにてどのページに遷移したかを取得する

実装案 (概略図)

model (rails)

Query (has many Pages)

  • query: string
  • session_id: string
  • その他諸々の検索オプション (Get the params #9)

Pages (belongs to Query)

  • query_id: integer
  • query: string (must be identical with page.query.q)
  • session_id: string (must be identical with page.query.session_id)
  • url: string
  • rank: integer
  • start: integer (2ページ目以降の場合のオフセット) (google のurl の key名が start なので)

chrome_extension

background.js

  • session_id = generate_session_id()
  • post_query(query, session_id)
  • send_session_id_to_content_script(session_id)

content_script.js

  • recieve_session_id_from_background(session_id)
  • post_url(query, session_id, url, rank)

TODO (実装順)

  • post_query(query) を post_query(query, session_id) に変更 (session_id は分離する)
    • queries controller 修正
  • pages を保存できるようにする
    • pages model 作成
    • pages controller 作成
    • post_url(query, session_id, url, rank) 実装
  • send_session_id_to_content_script(session_id) 実装 ( Queries モデルと Pages モデルの紐付け)
    • content_script.js から background.js に session_id を訊いて、レスポンスを貰う
  • リファクタリング
  • 試験運用

実装後

リファクタリング

  • chrome_extension
    • post系の関数を別jsファイルにまとめてもいいかも
    • オブジェクト指向
    • 細かい関数 (random_id() や generate_session_id() など )を別ファイルに分離

テスト

  • rails
    • そろそろテスト書かねば

@daicche
Copy link
Contributor Author

daicche commented Jun 24, 2014

@sosuke-k @taisho6339 ちょっと長いけど仕様レビューお願いします 🙇

@daicche
Copy link
Contributor Author

daicche commented Jun 24, 2014

@sosuke-k @taisho6339 分からないところは適宜飛ばすなり訊くなりしてください 🐾

@sosuke-k
Copy link

post_url(query, session_id, url, rank)
or
post_url(query, session_id, url, rank, paginated)

paginatedは?

@daicche
Copy link
Contributor Author

daicche commented Jun 24, 2014

例えば10件/ページの時に2ページ目に飛ぶと、URLが

https://www.google.co.jp/webhp?sourceid=chrome-instant&ion=1&espv=2&es_th=1&ie=UTF-8#q=hoge&safe=off&start=10

になって 最後の start=10 から、rankがn番目ってのは分かるからページ番号は取らなくてもいいかな、と思ってる

rankだけじゃなくてページ番号も取った方がいいかな?

@taisho6339
Copy link

とりあえずlgtm
not [beling] but [belong]

@sosuke-k
Copy link

う〜ん、ページ番号を使う場面すぐ思いつかない。

あれか、ページ番号ってマウスの位置とかスクロール量とかと同じ感じか。
まあとりあえず、いらないか。

@daicche
Copy link
Contributor Author

daicche commented Jun 25, 2014

@taisho6339 typo恥ずかしい…! 直しました 💃

@daicche
Copy link
Contributor Author

daicche commented Jun 25, 2014

@sosuke-k

ページ番号ってマウスの位置とかスクロール量とかと同じ感じか。

まさにそれ! GUI情報というかなんというかそんな感じ

@daicche
Copy link
Contributor Author

daicche commented Jun 25, 2014

実装開始します 🚋

@daicche daicche mentioned this pull request Jun 25, 2014
@daicche
Copy link
Contributor Author

daicche commented Jun 25, 2014

undefined keys が入ってきたときにエラー吐くから revert した 23ba9cd

@daicche
Copy link
Contributor Author

daicche commented Jun 25, 2014

ひとまずページ情報取れるようにした
Create prototype of Getting URLs 46fb223


add_binding_to_page()

// for ajax
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

検索結果ページの検索窓で再検索したときにページ遷移が起こらず(ajax)
content_script.jsが走らないので、 そのajax用の処理を追加

cf.
javascript - Chrome Extension: How to reload/re-execute content script on Ajax request - Stack Overflow
http://stackoverflow.com/questions/7325701/chrome-extension-how-to-reload-re-execute-content-script-on-ajax-request

@daicche
Copy link
Contributor Author

daicche commented Jun 25, 2014

@sosuke-k @taisho6339 長くなりそうなので一旦レビューお願いします 👓 👓 👓
どこまで実装したか は 上の TODO(実装順) を見てください

} else {
var query = parsed_url.hash.slice(1)
}
var page_data = query + '&url=' + clicked_url + '&rank=' + i
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rank が 3 で start が 10 の場合、本当の rank は 10 + 3 = 13 になるから、
rank のカラム名は変えた方がいいかも。 index とか…?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

サーバーにpostするのは本当のrankの方かと思ってたけど?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

こういう拡張とはうまくいかないんだよね?
https://chrome.google.com/webstore/detail/autopagerize/igiofjhpmpihnifddepnpngfjhkfenbp?hl=ja

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

サーバーにpostするのは本当のrankの方かと思ってたけど?

了解す。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

autoPagialize 的なのは普通に取得できた
https://github.com/re-born/queristory/blob/get-urls/extention/src/content_script.js#L6

で全部のページのリンク取得できてるっぽい

@daicche
Copy link
Contributor Author

daicche commented Jun 30, 2014

getting url で、一回のページ遷移でPOSTが複数回行われる · Issue #16 · re-born/queristory
#16

という問題に対応 26d8449

@daicche
Copy link
Contributor Author

daicche commented Jun 30, 2014

ページURLだけじゃなくて title も取得するように変更 92efd43

} else {
var query = parsed_url.hash.slice(1)
}
chrome.extension.sendRequest({greeting: 'get_session_id'}, function(response) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

background.js に session_id を尋ねる

@daicche
Copy link
Contributor Author

daicche commented Jun 30, 2014

pagination 追加 e10ca4f

@sosuke-k
Copy link

うぃ

@daicche
Copy link
Contributor Author

daicche commented Jun 30, 2014

Kaminari の部分のコードが汚かったので default_scope で対応 263aff2

@daicche
Copy link
Contributor Author

daicche commented Jun 30, 2014

wrong number of arguments (2 for 1) in site.sass · Issue #155 · Compass/compass-rails
Compass/compass-rails#155
と同じエラーを吐いてて辛かったので rails の version を上げて対応 3d27b73

@daicche
Copy link
Contributor Author

daicche commented Jul 2, 2014

before

after

@daicche
Copy link
Contributor Author

daicche commented Jul 3, 2014

検索オプションの表示 (['tbm', 'as_qdr', 'lr', 'tbs', 'source']の5つ) 0ba6238

@daicche
Copy link
Contributor Author

daicche commented Jul 3, 2014

tbm: ischとかじゃなくて "画像検索" にした方がええんかなぁ

@daicche
Copy link
Contributor Author

daicche commented Jul 6, 2014

viewの修正 8cc4689

before

after

daicche added a commit that referenced this pull request Jul 14, 2014
@daicche daicche merged commit 870bc97 into master Jul 14, 2014
@daicche daicche deleted the get-urls branch July 14, 2014 11:36
@daicche daicche changed the title [WIP]Design getting urls Design getting urls Oct 31, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants