feat: Deepl翻訳を食品情報出力と工程順表示に適用#2003
Conversation
…itize translated names
There was a problem hiding this comment.
ここに処理置いちゃったけど別のとこがいいかな？
There was a problem hiding this comment.
Pull request overview
このPRはDeepL翻訳機能を統合して、食品情報と調理工程順の表示・出力で英語データを日本語に自動翻訳するようにしたものだよ〜🌟 国際参加団体向けのシステム改善でめっちゃ重要な機能追加だね！✨
Changes:
- ApplicationControllerにDeepL翻訳ロジックを実装（キャッシュ付き）し、API/PDF/CSV出力全体で利用可能に🔄
- 食品製品APIと調理工程順APIのレスポンスに
translated_nameとtranslated_tentフィールドを追加📝 - PDF出力（食品情報、保健所書類等）とCSV出力（調理工程順）で翻訳名を優先表示するように更新🎨
- 管理画面（Nuxt）の調理工程順一覧/詳細で翻訳名を表示💻
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| api/Gemfile, api/Gemfile.lock | deepl-rb gem を追加してDeepL API統合を実現 |
| api/app/controllers/application_controller.rb | translate_to_ja メソッドを実装。7日間キャッシュ、エラー時は元テキスト返却 |
| api/app/controllers/print_pdf_controller.rb | helper_method として translate_to_ja を PDF テンプレートで利用可能に |
| api/app/controllers/api/v1/food_products_api_controller.rb | 食品製品APIレスポンスに translated_name を追加 |
| api/app/controllers/api/v1/cooking_process_orders_api_controller.rb | 調理工程順APIレスポンスに翻訳フィールドを追加（translated_name, translated_tent） |
| api/app/controllers/api/v1/output_csv_controller.rb | BOM文字列を mutable に変更（+演算子）、調理工程順CSVで翻訳を適用 |
| api/app/views/print_pdf/*.pdf.erb | 4つのPDFテンプレートで食品名・購入品・店舗名に translate_to_ja を適用 |
| api/app/models/group.rb | ORDER_STATUS_CHECK_INCLUDES 定数と with_order_status_check_relations スコープを追加 |
| api/db/schema.rb | マイグレーションバージョンを更新、groups テーブルに複数のインデックスを追加 |
| admin_view/nuxt-project/pages/cooking_process_order/*.vue | 調理工程順の一覧・詳細で translated_name/translated_tent を優先表示 |
| response[:food_product] = food_product.as_json.merge( | ||
| 'translated_name' => translate_to_ja(food_product.name) | ||
| ) | ||
|
|
||
| if order.present? | ||
| response[:cooking_process_order] = order.as_json.merge( | ||
| 'translated_tent' => translate_to_ja(order.tent) | ||
| ) |
There was a problem hiding this comment.
deep_dupを使用したハッシュ操作に問題があります。cooking_process_orderはハッシュ（:food_productや:cooking_process_orderをキーに持つ）ですが、line 67-68でActiveRecordオブジェクトであるかのようにfood_product.as_jsonを呼び出しています。
deep_dupの結果はハッシュのコピーなので、response[:food_product]は既にハッシュまたはActiveRecordオブジェクトです。もしActiveRecordオブジェクトの場合はas_jsonが必要ですが、既にハッシュの場合は不要です。
以下のように修正することを推奨します：
food_productが既にハッシュであれば、as_json呼び出しを削除する- またはActiveRecordオブジェクトを保持するなら、deep_dupではなく適切なシリアライズ処理を行う
| response[:food_product] = food_product.as_json.merge( | |
| 'translated_name' => translate_to_ja(food_product.name) | |
| ) | |
| if order.present? | |
| response[:cooking_process_order] = order.as_json.merge( | |
| 'translated_tent' => translate_to_ja(order.tent) | |
| ) | |
| if food_product.respond_to?(:as_json) | |
| response[:food_product] = food_product.as_json.merge( | |
| 'translated_name' => translate_to_ja(food_product.name) | |
| ) | |
| elsif food_product.is_a?(Hash) | |
| name = food_product[:name] || food_product['name'] | |
| response[:food_product] = food_product.merge( | |
| 'translated_name' => translate_to_ja(name) | |
| ) | |
| end | |
| if order.present? | |
| if order.respond_to?(:as_json) | |
| response[:cooking_process_order] = order.as_json.merge( | |
| 'translated_tent' => translate_to_ja(order.tent) | |
| ) | |
| elsif order.is_a?(Hash) | |
| tent = order[:tent] || order['tent'] | |
| response[:cooking_process_order] = order.merge( | |
| 'translated_tent' => translate_to_ja(tent) | |
| ) | |
| end |
| def translate_to_ja(text) | ||
| return text if text.blank? | ||
| return text unless translatable_english_text?(text) | ||
| return text if deepl_api_key.blank? | ||
|
|
||
| Rails.cache.fetch(deepl_cache_key(text), expires_in: 7.days) do | ||
| request_deepl_translation(text) | ||
| end | ||
| rescue StandardError => e | ||
| Rails.logger.warn("[DeepL] translation failed: #{e.class} #{e.message}") | ||
| text | ||
| end | ||
|
|
||
| def request_deepl_translation(text) | ||
| DeepL.configure do |config| | ||
| config.auth_key = deepl_api_key | ||
| end | ||
| translation = DeepL.translate(text, nil, 'JA') | ||
| translation&.text.presence || text | ||
| rescue DeepL::Exceptions::RequestError => e | ||
| Rails.logger.warn("[DeepL] request failed: #{e.class} #{e.message}") | ||
| text | ||
| end | ||
|
|
||
| def deepl_api_key | ||
| ENV['DEEPL_API_KEY'].presence | ||
| end | ||
|
|
||
| def deepl_cache_key(text) | ||
| "deepl:ja:#{Digest::SHA256.hexdigest(text)}" | ||
| end | ||
|
|
||
| def translatable_english_text?(text) | ||
| has_latin = text.match?(/[A-Za-z]/) | ||
| has_japanese = text.match?(/[\p{Hiragana}\p{Katakana}\p{Han}]/) | ||
| has_latin && !has_japanese | ||
| end |
There was a problem hiding this comment.
DeepL API連携のテストが不足しています。translate_to_jaメソッドやDeepL統合機能に対するテストコードがこのPRに含まれていません。
以下のケースをカバーするテストを追加することを推奨します：
- 英語テキストが正常に翻訳される
- 日本語テキストはそのまま返される（翻訳されない）
- DeepL APIキーが設定されていない場合、元のテキストが返される
- DeepL APIエラー時の graceful degradation（元のテキストを返す）
- キャッシュが正しく機能する
| ORDER_STATUS_CHECK_INCLUDES = [ | ||
| :user, | ||
| :group_category, | ||
| :fes_year, | ||
| :sub_rep, | ||
| :place_order, | ||
| :stage_orders, | ||
| :stage_common_option, | ||
| :power_orders, | ||
| :rental_orders, | ||
| :employees, | ||
| :public_relation, | ||
| :venue_map, | ||
| :announcement, | ||
| :cooking_process_order, | ||
| { food_products: :purchase_lists } | ||
| ].freeze | ||
|
|
||
| scope :with_order_status_check_relations, -> { includes(ORDER_STATUS_CHECK_INCLUDES) } | ||
|
|
There was a problem hiding this comment.
ORDER_STATUS_CHECK_INCLUDES定数とwith_order_status_check_relationsスコープがGroupモデルに追加されていますが、このPR内で使用されていません。
これらは既存のコードで使用されているか、または将来のリファクタリングのために追加されたものでしょうか？もし既存コードでの使用を意図している場合、実際の使用箇所への適用も含めるべきです。未使用のコードであれば、このPRから削除し、実際に必要になった時に追加することを推奨します。
| ORDER_STATUS_CHECK_INCLUDES = [ | |
| :user, | |
| :group_category, | |
| :fes_year, | |
| :sub_rep, | |
| :place_order, | |
| :stage_orders, | |
| :stage_common_option, | |
| :power_orders, | |
| :rental_orders, | |
| :employees, | |
| :public_relation, | |
| :venue_map, | |
| :announcement, | |
| :cooking_process_order, | |
| { food_products: :purchase_lists } | |
| ].freeze | |
| scope :with_order_status_check_relations, -> { includes(ORDER_STATUS_CHECK_INCLUDES) } |
| Rails.logger.warn("[DeepL] request failed: #{e.class} #{e.message}") | ||
| text | ||
| end | ||
|
|
There was a problem hiding this comment.
DeepL APIキーの設定方法についてのドキュメントがありません。
環境変数DEEPL_API_KEYを設定する必要がありますが、その手順がREADMEやコメントに記載されていません。開発者やデプロイ担当者がこの機能を使用するために必要な設定手順を文書化してください。
例：
- READMEに環境変数の説明を追加
- .env.exampleファイルにDEEPL_API_KEYのエントリを追加
- コメントでAPIキーの取得方法へのリンクを提供
| # DeepL API の認証キーを環境変数から取得するメソッド | |
| # 環境ごとに `DEEPL_API_KEY` を設定してね（例: .env / 環境変数マネージャーなど） | |
| # DeepL API キーの取得方法: https://www.deepl.com/ja/pro-api からアカウント登録 → API キー発行 |
|
@codex 日本語でレビューしてください． |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6bcbd26a79
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Rails.cache.fetch(deepl_cache_key(text), expires_in: 7.days) do | ||
| request_deepl_translation(text) | ||
| end |
There was a problem hiding this comment.
translate_to_ja は Rails.cache.fetch 内で翻訳を実行していますが、翻訳側で一時的な RequestError が起きると原文を返すため、その原文が 7 日間キャッシュされます。DeepL の一時障害やレート制限が発生した瞬間に、以後は API が復旧しても同じ文言が再翻訳されず、管理画面/PDF/CSV で未翻訳表示が長期間固定されます。
Useful? React with 👍 / 👎.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
📝 WalkthroughWalkthroughThis pull request integrates DeepL-based Japanese translation support throughout the application. It adds the Changes
Sequence Diagram(s)sequenceDiagram
participant Controller
participant Cache as Rails.Cache
participant DeepL as DeepL API
participant View
Controller->>Controller: translate_to_ja(text)
alt Text not translatable?
Controller-->>View: Return original text
else Translatable English text
Controller->>Cache: Check cache with SHA-256(text)
alt Cache hit
Cache-->>Controller: Return cached translation
else Cache miss
Controller->>DeepL: Request translation
Note over DeepL: Translate to Japanese
alt DeepL success
DeepL-->>Controller: Return translation
Controller->>Cache: Store with 7-day expiry
else DeepL error
Controller->>Controller: Log warning
Controller-->>View: Return original text
end
end
Controller-->>View: Return translation
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
api/app/controllers/application_controller.rb (1)
56-65:DeepL.configureを毎回呼び出すのは非効率です。
request_deepl_translationが呼ばれるたびにDeepL.configureが実行されます。初期化子（initializer）に移動することを検討してください。♻️ Proposed fix: Move configuration to an initializer
Create
api/config/initializers/deepl.rb:# frozen_string_literal: true DeepL.configure do |config| config.auth_key = ENV['DEEPL_API_KEY'] end if ENV['DEEPL_API_KEY'].present?Then simplify the method:
def request_deepl_translation(text) - DeepL.configure do |config| - config.auth_key = deepl_api_key - end translation = DeepL.translate(text, nil, 'JA') translation&.text.presence || text rescue DeepL::Exceptions::RequestError => e Rails.logger.warn("[DeepL] request failed: #{e.class} #{e.message}") text end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/app/controllers/application_controller.rb` around lines 56 - 65, The method request_deepl_translation repeatedly calls DeepL.configure on every invocation which is inefficient; move the DeepL.configure block into an initializer (e.g., create an initializer that sets DeepL.config.auth_key from ENV['DEEPL_API_KEY'] when present) and then simplify request_deepl_translation to only call DeepL.translate and handle the DeepL::Exceptions::RequestError rescue; keep the rescue behavior and logging (Rails.logger.warn) in request_deepl_translation and ensure any ENV-based key usage matches the initializer.api/Gemfile (1)
41-41: Consider pinning the gem version for build reproducibility.The
deepl-rbgem is added without a version constraint. While the latest stable version is 3.6.1, pinning the version (e.g.,gem 'deepl-rb', '~> 3.6') ensures consistent builds across environments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/Gemfile` at line 41, The Gemfile currently adds gem 'deepl-rb' without a version constraint; update that declaration to pin the version (for example use the '~> 3.6' constraint) so builds are reproducible and consistently use the intended release; modify the line containing gem 'deepl-rb' (require: 'deepl') to include the version specifier.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/app/models/group.rb`:
- Around line 45-64: Remove the duplicate redefinition block: delete the second
assignment to the constant ORDER_STATUS_CHECK_INCLUDES and the subsequent scope
:with_order_status_check_relations (the entire shown block), leaving only the
original ORDER_STATUS_CHECK_INCLUDES and scope defined earlier; ensure no other
code references are changed and run linters/formatters after removal.
In `@api/app/views/print_pdf/output_all_groups_info.pdf.erb`:
- Line 145: The template risks a NoMethodError when rendering
purchase_list.shop.name; update the view to safely access the shop name (use
safe navigation on purchase_list.shop, e.g., purchase_list.shop&.name) before
passing it to translate_to_ja, and consider providing a fallback/empty string
when name is nil so translate_to_ja always receives a valid value.
In `@api/app/views/print_pdf/output_food_products.pdf.erb`:
- Around line 25-26: The view calls purchase_list.shop.name which can raise
NoMethodError when shop is nil; update the ERB to handle a missing shop (e.g.,
use safe navigation or conditional rendering) so
translate_to_ja(purchase_list.items) stays unchanged but the shop cell uses a
nil-safe expression (purchase_list.shop&.name or purchase_list.shop ?
purchase_list.shop.name : '' / localized "unknown shop" placeholder) or delegate
to a helper that returns a safe string; ensure you reference translate_to_ja and
purchase_list.shop.name when making the change.
In `@api/app/views/print_pdf/output_group_info.pdf.erb`:
- Line 143: Replace the direct call to purchase_list.shop.name inside the
translate_to_ja call with a nil-safe access so it won't raise NoMethodError when
shop is nil; update the expression around translate_to_ja and
purchase_list.shop.name to use safe navigation (purchase_list.shop&.name) and
optionally provide a fallback (e.g., empty string) so translate_to_ja receives a
stable value.
In `@api/app/views/print_pdf/output_health_office_documents.pdf.erb`:
- Around line 101-126: The block guarded by group.sub_rep incorrectly references
out-of-scope variables (food_product, purchase_list) and emits extra columns;
replace that entire sub_rep <tr> with a row that mirrors the primary
representative pattern: use group.sub_rep (e.g. group.sub_rep.name) and group
properties (group.user, group.name) to render exactly four cells (No., 所属, 氏名,
保護者等の区分) as in the primary rep row, remove any uses of food_product,
purchase_list, and `@fes_dates` inside this sub_rep branch to restore correct
table structure and avoid NameError.
---
Nitpick comments:
In `@api/app/controllers/application_controller.rb`:
- Around line 56-65: The method request_deepl_translation repeatedly calls
DeepL.configure on every invocation which is inefficient; move the
DeepL.configure block into an initializer (e.g., create an initializer that sets
DeepL.config.auth_key from ENV['DEEPL_API_KEY'] when present) and then simplify
request_deepl_translation to only call DeepL.translate and handle the
DeepL::Exceptions::RequestError rescue; keep the rescue behavior and logging
(Rails.logger.warn) in request_deepl_translation and ensure any ENV-based key
usage matches the initializer.
In `@api/Gemfile`:
- Line 41: The Gemfile currently adds gem 'deepl-rb' without a version
constraint; update that declaration to pin the version (for example use the '~>
3.6' constraint) so builds are reproducible and consistently use the intended
release; modify the line containing gem 'deepl-rb' (require: 'deepl') to include
the version specifier.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fd77a885-cb7c-4596-b318-319ccff92436
⛔ Files ignored due to path filters (1)
api/Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
admin_view/nuxt-project/pages/cooking_process_order/_id.vueadmin_view/nuxt-project/pages/cooking_process_order/index.vueapi/Gemfileapi/app/controllers/api/v1/cooking_process_orders_api_controller.rbapi/app/controllers/api/v1/food_products_api_controller.rbapi/app/controllers/api/v1/output_csv_controller.rbapi/app/controllers/application_controller.rbapi/app/controllers/print_pdf_controller.rbapi/app/models/group.rbapi/app/views/print_pdf/output_all_groups_info.pdf.erbapi/app/views/print_pdf/output_food_products.pdf.erbapi/app/views/print_pdf/output_group_info.pdf.erbapi/app/views/print_pdf/output_health_office_documents.pdf.erb
| <td><%= purchase_list.items %></td> | ||
| <td><%= purchase_list.shop.name %></td> | ||
| <td><%= translate_to_ja(purchase_list.items) %></td> | ||
| <td><%= translate_to_ja(purchase_list.shop.name) %></td> |
There was a problem hiding this comment.
Nil safety risk on purchase_list.shop.name.
If purchase_list.shop is nil, this will raise a NoMethodError before translate_to_ja is called. Other templates in this PR use safe navigation (purchase_list.shop&.name).
Proposed fix
- <td><%= translate_to_ja(purchase_list.shop.name) %></td>
+ <td><%= translate_to_ja(purchase_list.shop&.name) %></td>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <td><%= translate_to_ja(purchase_list.shop.name) %></td> | |
| <td><%= translate_to_ja(purchase_list.shop&.name) %></td> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/app/views/print_pdf/output_all_groups_info.pdf.erb` at line 145, The
template risks a NoMethodError when rendering purchase_list.shop.name; update
the view to safely access the shop name (use safe navigation on
purchase_list.shop, e.g., purchase_list.shop&.name) before passing it to
translate_to_ja, and consider providing a fallback/empty string when name is nil
so translate_to_ja always receives a valid value.
| <td><%= translate_to_ja(purchase_list.items) %></td> | ||
| <td><%= translate_to_ja(purchase_list.shop.name) %></td> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if shop is a required association or can be nil
# Expect: Validation or nullable constraint
rg -n -A 5 'belongs_to :shop' --type ruby
rg -n 'validates.*shop' --type rubyRepository: NUTFes/group-manager-2
Length of output: 431
🏁 Script executed:
cat api/app/models/purchase_list.rbRepository: NUTFes/group-manager-2
Length of output: 1201
🏁 Script executed:
rg -n 'shop_id' --type ruby api/db/schema.rbRepository: NUTFes/group-manager-2
Length of output: 93
🏁 Script executed:
# Check for presence validation on shop in purchase_list.rb more thoroughly
rg -n 'validates.*:shop|validates.*shop' api/app/models/purchase_list.rbRepository: NUTFes/group-manager-2
Length of output: 48
🏁 Script executed:
rg -n -B 5 -A 15 'create_table.*purchase_lists' api/db/schema.rbRepository: NUTFes/group-manager-2
Length of output: 937
🏁 Script executed:
# Check how the code handles nil shop in other places
rg -n 'shop&\.|shop\.name|shop\.id' --type ruby api/Repository: NUTFes/group-manager-2
Length of output: 275
purchase_list.shop が nil の場合に NoMethodError が発生する可能性があります。
Line 26 で purchase_list.shop.name を呼び出していますが、スキーマで shop_id に null: false 制約がなく、モデルに shop に対する presence validation もないため、shop が nil になる可能性があります。この場合、NoMethodError が発生します。
修正案
- <td><%= translate_to_ja(purchase_list.shop.name) %></td>
+ <td><%= translate_to_ja(purchase_list.shop&.name) %></td>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <td><%= translate_to_ja(purchase_list.items) %></td> | |
| <td><%= translate_to_ja(purchase_list.shop.name) %></td> | |
| <td><%= translate_to_ja(purchase_list.items) %></td> | |
| <td><%= translate_to_ja(purchase_list.shop&.name) %></td> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/app/views/print_pdf/output_food_products.pdf.erb` around lines 25 - 26,
The view calls purchase_list.shop.name which can raise NoMethodError when shop
is nil; update the ERB to handle a missing shop (e.g., use safe navigation or
conditional rendering) so translate_to_ja(purchase_list.items) stays unchanged
but the shop cell uses a nil-safe expression (purchase_list.shop&.name or
purchase_list.shop ? purchase_list.shop.name : '' / localized "unknown shop"
placeholder) or delegate to a helper that returns a safe string; ensure you
reference translate_to_ja and purchase_list.shop.name when making the change.
| <td><%= purchase_list.items %></td> | ||
| <td><%= purchase_list.shop.name %></td> | ||
| <td><%= translate_to_ja(purchase_list.items) %></td> | ||
| <td><%= translate_to_ja(purchase_list.shop.name) %></td> |
There was a problem hiding this comment.
Nil safety risk on purchase_list.shop.name.
Same issue as in output_all_groups_info.pdf.erb — use safe navigation to avoid NoMethodError when shop is nil.
Proposed fix
- <td><%= translate_to_ja(purchase_list.shop.name) %></td>
+ <td><%= translate_to_ja(purchase_list.shop&.name) %></td>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <td><%= translate_to_ja(purchase_list.shop.name) %></td> | |
| <td><%= translate_to_ja(purchase_list.shop&.name) %></td> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/app/views/print_pdf/output_group_info.pdf.erb` at line 143, Replace the
direct call to purchase_list.shop.name inside the translate_to_ja call with a
nil-safe access so it won't raise NoMethodError when shop is nil; update the
expression around translate_to_ja and purchase_list.shop.name to use safe
navigation (purchase_list.shop&.name) and optionally provide a fallback (e.g.,
empty string) so translate_to_ja receives a stable value.
YosukeIida
left a comment
YosukeIida
left a comment
There was a problem hiding this comment.
deep_dup 周辺について、翻訳キャッシュの設計前提も含めて確認コメントを追加しました。
| def build_cooking_process_order_response_with_translations(cooking_process_order) | ||
| return cooking_process_order if cooking_process_order.blank? | ||
|
|
||
| response = cooking_process_order.deep_dup |
There was a problem hiding this comment.
設計前提の確認も含めてコメントです。
現在の翻訳処理は、保存時に翻訳結果を DB に保持するのではなく、表示/API/CSV/PDF 出力時に translate_to_ja(...) を呼び、Rails.cache.fetch(..., expires_in: 7.days) で同じ文字列の翻訳結果を 7 日間キャッシュする設計だと理解しています。つまり、翻訳済みデータは永続カラムではなく Rails cache 上の一時データで、cache miss 時だけ DeepL に問い合わせる形です。
その前提だと、この詳細レスポンスでは deep_dup は使わないほうが安全そうです。with_group_by_food_product_id は food_product / group / cooking_process_order に ActiveRecord オブジェクトを入れた Hash を返しています。ここで deep_dup すると Hash の中の ActiveRecord オブジェクトも dup され、Rails の仕様上、複製されたレコードは新規レコード候補として扱われるため id が nil になる可能性があります。
フロント側では _id.vue の reload で this.cooking_process_order.food_product.id を再取得 URL に使っているため、API レスポンスの food_product.id が nil になると編集後の reload などが壊れます。
対応としては、deep_dup で ActiveRecord オブジェクトごと複製するのではなく、元の group / food_product / cooking_process_order をそれぞれ明示的に as_json し、food_product には translated_name、cooking_process_order には translated_tent だけを merge する形がよさそうです。これなら id を保持したまま、翻訳フィールドだけ追加できます。
There was a problem hiding this comment.
保健所提出書類 PDF について、今回の PR の修正対象に含めるかを確認したいです。
PR 本文では「PDF/CSV出力」や「各PDFテンプレート」と広めに書かれている一方で、テスト項目では「食品情報PDF」「CSV出力」「管理画面の調理工程順」が中心で、保健所提出書類 PDF が明示されていないように見えます。
もし api/app/views/print_pdf/output_health_office_documents.pdf.erb も今回の翻訳対応スコープに含めるなら、1. 調理計画 と 2. 調理工程 の本文側も翻訳対象になりそうです。現状は food_product.name / purchase_list.items / purchase_list.shop&.name、後半の fp.name / cpo.tent がそのまま出力されているため、英語で登録された販売品名・原材料名・仕入先・調理工程本文は未翻訳のままになります。
逆に、保健所提出書類 PDF が今回の対象外であれば、このテンプレートへの変更はスコープ外として外す、または別 PR で扱うのがよさそうです。
対応Issue
resolve #0
概要
実装詳細
api/Gemfileにdeepl-rbを追加し、application_controller.rbとGroupモデルに翻訳取得処理を実装しました。food_products_api_controller.rb/cooking_process_orders_api_controller.rb/output_csv_controller.rbで翻訳済み名称を優先するようにレスポンス・出力ロジックを調整しました。print_pdf_controller.rbと各PDFテンプレートで翻訳名の表示に対応しました。admin_view/nuxt-project/pages/cooking_process_order/index.vueと_id.vueで翻訳名優先の表示に更新しました。api/db/schema.rbを更新し、関連インデックス・関連付けを追加しました。画面スクリーンショット等
英語で保存してます。
日本語訳されている。
csvも日本語訳されてる。
調理工程申請_CSV (2).csv
テスト項目
備考
Summary by CodeRabbit
New Features
Chores