Skip to content

算定基礎届の総計が7桁を超える場合9999999に丸める#69

Merged
wakasa51 merged 2 commits intokufu:masterfrom
sugamasao:fix-DataRecord2225700-income_all_total
Jul 7, 2020
Merged

算定基礎届の総計が7桁を超える場合9999999に丸める#69
wakasa51 merged 2 commits intokufu:masterfrom
sugamasao:fix-DataRecord2225700-income_all_total

Conversation

@sugamasao
Copy link
Copy Markdown
Contributor

https://www.nenkin.go.jp/denshibenri/oshirase/zenpan/20190820.files/14.pdf
該当は項目36の総計欄です。
項目37の平均と同じように7桁へ丸める処理を追加しています。

@sugamasao sugamasao requested a review from a team as a code owner July 7, 2020 06:31
@sugamasao sugamasao requested review from motsat and tknzk and removed request for a team July 7, 2020 06:31

# 総計を計算する
# CSVへ出力する際、10,000,000 円以上は 9,999,999 を返す
def income_all_total_with_round
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

income_all_total メソッド自体は平均額の計算などでも利用しているので、丸める処理のみを行うメソッドを追加しています

Copy link
Copy Markdown
Member

@wakasa51 wakasa51 left a comment

Choose a reason for hiding this comment

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

LGTM!! 👍

before do
allow(rec).to receive(:target_months).and_return(%i[apr may jun])
end
context 'not round' do
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[nits] 個人的な意見ですが、 context には望む結果ではなく、条件的なものが入ると読みやすいかなと思います。

例えば、 when income_all_total is 9_999_999 or lesswhen income_all_total is more than 9_999_999 みたいな感じなど

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

確かにそうですね。
メッセージ直しますので少々お待ちを 🙏

Copy link
Copy Markdown
Contributor

@tknzk tknzk left a comment

Choose a reason for hiding this comment

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

:lgtm:

@sugamasao
Copy link
Copy Markdown
Contributor Author

rspec内のcontextのメッセージを修正しました。

自分でも検討してみたのですが、例示していただいたメッセージが一番良かったのでそのまま使わせていただきました ☘️

Copy link
Copy Markdown
Member

@wakasa51 wakasa51 left a comment

Choose a reason for hiding this comment

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

対応ありがとうございます!
再LGTM!! 👍

@wakasa51 wakasa51 merged commit b0818f3 into kufu:master Jul 7, 2020
@sugamasao sugamasao deleted the fix-DataRecord2225700-income_all_total branch July 7, 2020 07:27
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.

3 participants