-
-
Notifications
You must be signed in to change notification settings - Fork 108
has_one
-> has_many
between Dojo and DojoEventService
#182
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
Merged
+57
−51
Merged
Changes from 2 commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,23 +19,19 @@ namespace :dojo_event_services do | |
next | ||
end | ||
|
||
insert = false | ||
unless dojo.dojo_event_service | ||
dojo.build_dojo_event_service | ||
insert = true | ||
end | ||
|
||
dojo.dojo_event_service.assign_attributes(des.except('dojo_id')) | ||
if dojo.dojo_event_service.changed? | ||
changes = dojo.dojo_event_service.changes | ||
dojo.dojo_event_service.save! | ||
(insert ? result[:inserted] : result[:updated]) << [des['dojo_id'], changes] | ||
dojo_event_service = dojo.dojo_event_services.find_or_initialize_by(des) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
if dojo_event_service.changed? | ||
changes = dojo_event_service.changes | ||
new_record = dojo_event_service.new_record? | ||
dojo_event_service.save! | ||
(new_record ? result[:inserted] : result[:updated]) << ["#{des['dojo_id']}:#{dojo_event_service.id}", changes] | ||
else | ||
result[:kept] << [des['dojo_id']] | ||
result[:kept] << ["#{des['dojo_id']}:#{dojo_event_service.id}"] | ||
end | ||
end | ||
|
||
# Dump result | ||
result[:skipped] = result[:skipped].uniq {|s| s.first } | ||
result.except!(:kept, :skipped) unless ENV.key?('DEBUG') | ||
sorted = result.sort_by {|_, v| v.length }.reverse.to_h | ||
puts | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
月末のイベントが翌月に重複していたので、UTC時刻で範囲指定する必要があるようです。
ただ、リファレンスが見つからずちょっと確証がないです。
事象としては、
2013年6月30日(日) 13:30-14:30のCoderDojoTokyoのイベントが、6月集計分と7月集計分のAPIのレスポンスに含まれており、
7月集計分をinsertする際にユニーク制約でARがraiseしていました。
(9時間ズレなら15時なんですよねぇ。。。)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
h[:since] = since_at.to_i + (3600 * 6) if since_at
試しに1時間ずつ時刻をズラしていって6時間オフセットしたらエラーが出なくなった。
2013/07/01 00:00:00+09:00
がsince_atに入っている時刻なので、と同義であり、UTCでも6/30の扱いのように見える。
なんだろうこれ、検索時のタイムゾーンはUTCでもないということなのか🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://developers.facebook.com/docs/graph-api/reference/v2.10/group/events
タイムゾーンについては特に書かれていなかった。
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
一次情報がみつからなくてちょっと真偽がはっきりしないんですが以下の記事を見つけました。
https://trim.bz/2013/01/24/1799
ほほぅ🤔ってことで、
#182 (comment) について検証してみました。
「2013年6月30日(日) 13:30-14:30のCoderDojoTokyoのイベント」の開始が13:30であることから、5時間のオフセットだと検索に引っかかり、6時間のオフセットだと検索にひっかからなくなる、というのがこれで説明がつくことがわかりました。
つまり、フェイスブックのAPIにはPST(RubyではPDT -7:00?)のタイムゾーンで時刻を渡してやる必要がある、ということのようです。
PDTとJSTのオフセットは8時間なので
3600*8
を足した時刻のUNIXタイムスタンプをパラメータに渡してやると良さそう。There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yasulab @hanachin
ここだけレビューをお願いしたいです。🙏
問題無さそうならマージしてしまって、 #183 も取り込むので #184 までいけそうだなと考えています😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
コード確認しました! ✅ 👀 問題なさそうなので、マージしちゃいますね ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ここ難しいですね
「site:facebook.com timezone api」みたいな感じで検索するとPSTでつかわれてるというFacebookヘルプチームからの解答がひっかかりました。
ということなので、オフセット足してよいと思います。