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

Redmine4.1でcsvファイルがimportされないバグを修正 #11

Merged
merged 9 commits into from Feb 14, 2020

Conversation

@Kang-Yuchan
Copy link

Kang-Yuchan commented Feb 6, 2020

@Kang-Yuchan Kang-Yuchan requested a review from hanachin Feb 6, 2020
hanachin and others added 3 commits Feb 6, 2020
@@ -13,13 +13,13 @@ def encode_csv_data
self.csv_data = self.csv_data
# 入力文字コード
encode = case self.encoding
when "U"
when "UTF-8"

This comment has been minimized.

Copy link
@hanachin

hanachin Feb 6, 2020

cd81196 このコミットで csv_data の内容が常にUTF-8に変換されるようになったので、ここ変更せずにcsvに渡してるオプション削るだけでもよさそうです。

@@ -6,7 +6,8 @@

class ImporterController < ApplicationController
unloadable

skip_before_action :verify_authenticity_token

This comment has been minimized.

Copy link
@hanachin

hanachin Feb 6, 2020

CSRF検証するようになった原因がよくわかっていません。無造作にskipしていいのか不安です。

This comment has been minimized.

Copy link
@hanachin

hanachin Feb 6, 2020

RAILS_ENV=testを設定してテストを実行しないといけないことがわかった、orbでexecutorの環境変数を設定するようにした
agileware-jp/redmine-plugin-orb#33

@Kang-Yuchan Kang-Yuchan requested a review from hanachin Feb 6, 2020
@hanachin hanachin force-pushed the bug/fix_import_bug branch from c17aaf4 to 39bfc5d Feb 6, 2020
It is always UTF-8 because csv_data is converted by encode_csv_data.
@hanachin hanachin force-pushed the bug/fix_import_bug branch from 39bfc5d to f55afad Feb 6, 2020
Copy link

hanachin left a comment

よさそうなきがします

@kyamada23 kyamada23 merged commit 4af1850 into master Feb 14, 2020
2 checks passed
2 checks passed
ci/circleci: redmine41-download Your tests passed on CircleCI!
Details
ci/circleci: redmine41-test Your tests passed on CircleCI!
Details
@kyamada23 kyamada23 deleted the bug/fix_import_bug branch Feb 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.