weko#62759 Fix title auto-fill leaving the title blank for non-standard item types - #1912
Open
MakotoASAOKA wants to merge 1 commit into
Open
weko#62759 Fix title auto-fill leaving the title blank for non-standard item types#1912MakotoASAOKA wants to merge 1 commit into
MakotoASAOKA wants to merge 1 commit into
Conversation
Reviewer's GuideFix title auto-fill for item types with non-standard title subitem names by resolving keys from jpcoar mappings, passing them to the edit UI, and guarding language-field assignment against schemas that omit language subitems. Sequence diagram for dynamic title auto-fillsequenceDiagram
participant Workflow as display_activity()
participant Mapping as Mapping.get_record()
participant Template as item_edit.html
participant UI as autoSetTitle()
participant Model as invenioRecordsModel
Workflow->>Mapping: get_record(item_type_id)
Mapping-->>Workflow: jpcoar_mapping.title keys
Workflow->>Template: render title_subitem_key and title_language_subitem_key
Template-->>UI: hidden input values
UI->>UI: Read resolved keys with fallback
UI->>Model: Store generated title under titleSubKey
alt language subitem exists
UI->>Model: Store language value under titleLanguageKey
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
PR Summary by QodoFix title auto-fill for item-specific title schemas
AI Description
Diagram
High-Level Assessment
Files changed (4)
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Fixed security issues:
- Cross-site scripting (XSS) via untrusted HTML/JS injection in web rendering sinks (link)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="modules/weko-workflow/weko_workflow/views.py" line_range="822-826" />
<code_context>
+ for mapping_value in item_type_mapping.values():
+ if not isinstance(mapping_value, dict):
+ continue
+ jpcoar_title = (mapping_value.get('jpcoar_mapping') or {}).get('title')
+ if isinstance(jpcoar_title, dict) and jpcoar_title.get('@value'):
+ title_subitem_key = jpcoar_title.get('@value')
+ title_language_subitem_key = jpcoar_title.get(
+ '@attributes', {}).get('xml:lang') or ""
+ break
+ return title_subitem_key, title_language_subitem_key
+
</code_context>
<issue_to_address>
**issue (bug_risk):** A truthy non-dict `jpcoar_mapping` value causes `(mapping_value.get('jpcoar_mapping') or {}).get('title')` to raise `AttributeError`, and a non-dict `@attributes` value causes the subsequent `.get('@attributes', {}).get(...)` chain to raise as well. The helper is documented and tested as a safe fallback, but malformed mapping data makes the item-edit page fail instead of returning empty keys.
**Triggers:** When an item type mapping contains a non-empty malformed `jpcoar_mapping` or `@attributes` value.
**Suggested fix:** Check that `jpcoar_mapping` and `@attributes` are dictionaries before calling `.get()`, or catch malformed mapping data and retain the empty-key fallback.
```suggestion
jpcoar_mapping = mapping_value.get('jpcoar_mapping')
if not isinstance(jpcoar_mapping, dict):
continue
jpcoar_title = jpcoar_mapping.get('title')
if isinstance(jpcoar_title, dict) and jpcoar_title.get('@value'):
title_subitem_key = jpcoar_title.get('@value')
jpcoar_attributes = jpcoar_title.get('@attributes')
if isinstance(jpcoar_attributes, dict):
title_language_subitem_key = jpcoar_attributes.get(
'xml:lang') or ""
```
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and if the resolved subitem key or language key is wrong, auto-fill can write an incorrect title into saved item metadata, and reverting would not remove titles already saved. Those records remain editable and the values can be corrected or regenerated, so the impact is bounded and repairable.
Blocking findings: modules/weko-workflow/weko_workflow/views.py:826
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+822
to
+826
| jpcoar_title = (mapping_value.get('jpcoar_mapping') or {}).get('title') | ||
| if isinstance(jpcoar_title, dict) and jpcoar_title.get('@value'): | ||
| title_subitem_key = jpcoar_title.get('@value') | ||
| title_language_subitem_key = jpcoar_title.get( | ||
| '@attributes', {}).get('xml:lang') or "" |
There was a problem hiding this comment.
issue (bug_risk): A truthy non-dict jpcoar_mapping value causes (mapping_value.get('jpcoar_mapping') or {}).get('title') to raise AttributeError, and a non-dict @attributes value causes the subsequent .get('@attributes', {}).get(...) chain to raise as well. The helper is documented and tested as a safe fallback, but malformed mapping data makes the item-edit page fail instead of returning empty keys.
Triggers: When an item type mapping contains a non-empty malformed jpcoar_mapping or @attributes value.
Suggested fix: Check that jpcoar_mapping and @attributes are dictionaries before calling .get(), or catch malformed mapping data and retain the empty-key fallback.
Suggested change
| jpcoar_title = (mapping_value.get('jpcoar_mapping') or {}).get('title') | |
| if isinstance(jpcoar_title, dict) and jpcoar_title.get('@value'): | |
| title_subitem_key = jpcoar_title.get('@value') | |
| title_language_subitem_key = jpcoar_title.get( | |
| '@attributes', {}).get('xml:lang') or "" | |
| jpcoar_mapping = mapping_value.get('jpcoar_mapping') | |
| if not isinstance(jpcoar_mapping, dict): | |
| continue | |
| jpcoar_title = jpcoar_mapping.get('title') | |
| if isinstance(jpcoar_title, dict) and jpcoar_title.get('@value'): | |
| title_subitem_key = jpcoar_title.get('@value') | |
| jpcoar_attributes = jpcoar_title.get('@attributes') | |
| if isinstance(jpcoar_attributes, dict): | |
| title_language_subitem_key = jpcoar_attributes.get( | |
| 'xml:lang') or "" |
Code Review by Qodo
1. PR title lacks prefix
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| }; | ||
|
|
||
| $scope.isExistingTitle = function () { | ||
| // The subitem key used for an item type's title varies by item |
There was a problem hiding this comment.
1. Pr title lacks prefix 📘 Rule violation ⚙ Maintainability
The PR title weko#62759 Fix title auto-fill leaving the title blank for non-standard item types does not begin with a lowercase prefix followed by : . It therefore fails the required `^[a-z]+: \S.*$` format.
Comment on lines
+2255
to
+2258
| if (hasLanguageKey) { | ||
| enTitle[titleLanguageKey] = "en"; | ||
| jaTitle[titleLanguageKey] = "ja"; | ||
| } |
There was a problem hiding this comment.
2. app.js behavior lacks tests 📘 Rule violation ▣ Testability
The PR changes title lookup and introduces the hasLanguageKey branch in app.js, but the added tests only exercise the Python key resolver. The client-side behavior for non-standard keys and schemas without language fields therefore lacks regression coverage.
Agent Prompt
## Issue description
The modified automatic-title behavior in `app.js` is not covered by tests.
## Issue Context
Add regression cases for non-standard title subitem keys, existing-title detection, and title schemas that omit the language subitem key.
## Fix Focus Areas
- modules/weko-items-ui/weko_items_ui/static/js/weko_items_ui/app.js[2178-2259]
- modules/weko-items-ui/weko_items_ui/static/js/weko_items_ui/app.js[5284-5285]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| get_current_user | ||
| from weko_logging.activity_logger import UserActivityLogger | ||
| from weko_records.api import FeedbackMailList, RequestMailList, ItemLink, ItemTypes, ItemApplication | ||
| from weko_records.api import FeedbackMailList, RequestMailList, ItemLink, ItemTypes, ItemApplication, Mapping |
There was a problem hiding this comment.
3. mapping import is unsorted 📘 Rule violation ⚙ Maintainability
The newly added Mapping member is placed after ItemApplication rather than in alphabetic order. This violates the configured isort sorting requirement for from-import members.
Agent Prompt
## Issue description
The modified `weko_records.api` import does not alphabetize its imported members.
## Issue Context
The module's tox configuration uses isort with the Black profile. Run isort or manually place `Mapping` in the resulting alphabetical order.
## Fix Focus Areas
- modules/weko-workflow/weko_workflow/views.py[61-61]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+822
to
+826
| jpcoar_title = (mapping_value.get('jpcoar_mapping') or {}).get('title') | ||
| if isinstance(jpcoar_title, dict) and jpcoar_title.get('@value'): | ||
| title_subitem_key = jpcoar_title.get('@value') | ||
| title_language_subitem_key = jpcoar_title.get( | ||
| '@attributes', {}).get('xml:lang') or "" |
There was a problem hiding this comment.
4. Helper fails black formatting 📘 Rule violation ⚙ Maintainability
The newly added helper uses formatting that Black would rewrite, including single-quoted string literals. Consequently, black --check would not accept the modified Python code unchanged.
Agent Prompt
## Issue description
The added `get_title_subitem_keys()` implementation is not in Black-normalized form.
## Issue Context
The repository lint environment invokes `black .`; run the configured formatter and commit its changes to the modified Python files.
## Fix Focus Areas
- modules/weko-workflow/weko_work_workflow/views.py[800-828]
- modules/weko-workflow/tests/test_views.py[4631-4650]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
|
||
| return jsonify(res), 200 | ||
|
|
||
| def get_title_subitem_keys(item_type_id): |
There was a problem hiding this comment.
5. Missing function separation 📘 Rule violation ✧ Quality
The new top-level get_title_subitem_keys() definition has only one blank line after the preceding function. Flake8 reports this as E305, so the touched Python file does not pass static analysis.
Agent Prompt
## Issue description
The new top-level helper is not separated from the preceding function by two blank lines.
## Issue Context
Flake8 requires two blank lines before a top-level function definition and reports the current layout as `E305`.
## Fix Focus Areas
- modules/weko-workflow/weko_workflow/views.py[798-800]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
whose title subitem key isn't subitem_item_title autoSetTitle()/isExistingTitle()/updateTitleForOutputReport() in weko_items_ui/app.js hardcoded the title subitem key names (subitem_item_title / subitem_item_title_language). Item types whose title property uses a different subitem key (e.g. subitem_restricted_access_item_title, used by the restricted-access reference item type from scripts/demo/resticted_access.sql) never match `value.items.properties.hasOwnProperty(titleSubKey)`, so the autofilled title is silently never written into the record model and the title field is left blank. Resolve the subitem key names dynamically instead, from the item type's own item_type_mapping (the same jpcoar_mapping.title source weko_deposit.api.WekoDeposit.get_titles already uses), via a new weko_workflow.views.get_title_subitem_keys() passed through display_activity()'s render_template call and two new hidden inputs in item_edit.html (title_subitem_key / title_language_subitem_key), falling back to the standard subitem_item_title(/_language) names if nothing can be resolved. Also guard the language sub-key assignment in autoSetTitle() behind an explicit schema check: some title properties (e.g. item_1578299480500 on item types 3007/3008) declare no language sibling field at all, and unconditionally assigning one produced an array item the record could not be saved with. Adds weko_workflow/tests/test_views.py::test_get_title_subitem_keys(_no_mapping) covering the new helper.
API インベントリ差分（件数のみ）
ベースラインとの差分API インベントリ差分レポート
判定: ✅ PASS (FAIL 0 / WARN 2)サマリ
[WARN] W2 実装本体が変化(data_op / 情報露出を再確認) — 2件
[WARN] W6 依存パッケージの版が変化した — 40件
台帳との突き合わせスナップショット ↔ インベントリ 突き合わせ
判定: ✅ 一致 (0件)
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
状況
weko_items_ui/app.jsのautoSetTitle()/isExistingTitle()/updateTitleForOutputReport()が、タイトルのsubitemキー名をsubitem_item_title/subitem_item_title_languageに固定している。この名前を使わない item type (例:scripts/demo/resticted_access.sqlが登録する制限公開参照アイテムタイプのsubitem_restricted_access_item_title) では、value.items.properties.hasOwnProperty(titleSubKey)が常に false になり、タイトル自動設定機能でタイトルがレコードモデルに一切反映されず、保存後もタイトルが空欄のままになる。再現手順
subitem_item_title以外の名前でタイトルのsubitemを定義した item type を用意するWEKO_ITEMS_UI_AUTO_FILL_TITLE_SETTING等でこの item type に対しタイトル自動設定を有効にする併せて、item_type の title プロパティが言語サブキーを持たない場合(例:
item_1578299480500)、autoSetTitle()が無条件にenTitle[titleLanguageKey]/jaTitle[titleLanguageKey]を設定しようとし、配列要素がスキーマと不整合になって登録自体が失敗する問題も含む。回避策(修正前)
無し。タイトル自動設定機能に依存する item type ではタイトルを手動入力する必要があった。
改修範囲
modules/weko-workflow/weko_workflow/views.py: item_type_mapping のjpcoar_mapping.titleから実際のタイトルsubitemキー名・言語サブキー名を動的に解決するget_title_subitem_keys()を追加し、display_activity()のレンダリングコンテキストに渡すmodules/weko-items-ui/weko_items_ui/templates/weko_items_ui/iframe/item_edit.html: 上記の値を保持する隠しinputを2つ追加(title_subitem_key/title_language_subitem_key)modules/weko-items-ui/weko_items_ui/static/js/weko_items_ui/app.js: ハードコードされていた3箇所を、上記隠しinputから動的に取得する形に変更(値が無い場合は従来通りsubitem_item_title/subitem_item_title_languageにフォールバック)。あわせて、スキーマに言語サブキーが定義されている場合のみ設定するガードを追加modules/weko-workflow/tests/test_views.py:get_title_subitem_keys()の単体テストを追加(既存item_typeフィクスチャでの解決確認、マッピング無し/存在しないIDでの安全なフォールバック確認)。ローカルで実行し2件とも成功を確認済みブランチ名
fix/issue62759プルリクエストURL
(この PR 自体)
Summary by Sourcery
Make automatic title handling compatible with item-type-specific title schemas.
Bug Fixes:
Enhancements:
Tests: