Skip to content

Commit 8224856

Browse files
Reject no-op profile change requests
Signed-off-by: yoshifuminakamura <nakamura@riken.jp>
1 parent 9159887 commit 8224856

4 files changed

Lines changed: 165 additions & 17 deletions

File tree

result_server/routes/admin.py

Lines changed: 94 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,33 @@ def _profile_request_note_metadata(
200200
}
201201

202202

203+
def _profile_request_followup_metadata(
204+
source_profile: dict,
205+
*,
206+
note: str,
207+
desired_schedule: str = "",
208+
desired_watch_target: str = "",
209+
preserve_profile_metadata: bool = False,
210+
) -> dict:
211+
source_metadata = source_profile.get("metadata_json") or {}
212+
metadata = (
213+
dict(source_metadata)
214+
if preserve_profile_metadata and isinstance(source_metadata, dict)
215+
else {}
216+
)
217+
metadata.pop("request_note", None)
218+
desired_schedule = desired_schedule.strip()
219+
desired_watch_target = desired_watch_target.strip()
220+
if desired_schedule:
221+
metadata["desired_schedule"] = desired_schedule
222+
if desired_watch_target:
223+
metadata["desired_watch_target"] = desired_watch_target
224+
request_note = note.strip()
225+
if request_note:
226+
metadata["request_note"] = request_note
227+
return metadata
228+
229+
203230
def _parse_trigger_definition_form():
204231
"""Return a raw trigger definition object from the submitted admin form."""
205232
actor = session.get("user_email", "")
@@ -401,7 +428,6 @@ def _profile_request_change_rows(profile_request, source_profile):
401428
metadata_specs = [
402429
("Desired Schedule", "desired_schedule"),
403430
("Desired Watch Target", "desired_watch_target"),
404-
("Note", "note"),
405431
]
406432
for label, key in metadata_specs:
407433
current = current_metadata.get(key)
@@ -415,6 +441,18 @@ def _profile_request_change_rows(profile_request, source_profile):
415441
return rows
416442

417443

444+
def _profile_request_has_effective_changes(requested_profile, source_profile):
445+
return bool(
446+
_profile_request_change_rows(
447+
{
448+
"request_type": "change_profile",
449+
"requested_profile": requested_profile,
450+
},
451+
source_profile,
452+
)
453+
)
454+
455+
418456
def _profile_request_diff_key(value):
419457
if isinstance(value, (list, tuple)):
420458
return tuple(str(item).strip() for item in value if str(item).strip())
@@ -920,14 +958,23 @@ def submit_execution_profile_followup_request():
920958

921959
requested_profile = dict(source_profile)
922960
requested_profile["status"] = "draft"
923-
requested_profile["metadata_json"] = {
924-
**(source_profile.get("metadata_json") or {}),
925-
**_profile_request_note_metadata(
926-
note,
927-
desired_schedule=desired_schedule,
928-
desired_watch_target=desired_watch_target,
929-
),
930-
}
961+
requested_profile["metadata_json"] = _profile_request_followup_metadata(
962+
source_profile,
963+
note=note,
964+
desired_schedule=desired_schedule,
965+
desired_watch_target=desired_watch_target,
966+
preserve_profile_metadata=request_type == "change_profile",
967+
)
968+
if request_type == "change_profile" and not _profile_request_has_effective_changes(
969+
requested_profile,
970+
source_profile,
971+
):
972+
flash(
973+
"Execution profile follow-up request was not created: "
974+
"change request needs a schedule, watch target, or profile field change; "
975+
"note-only changes are not actionable"
976+
)
977+
return redirect(url_for("profile_requests.profile_requests"))
931978
try:
932979
request_id = store.create_profile_request(
933980
requested_profile=requested_profile,
@@ -975,6 +1022,44 @@ def resubmit_execution_profile_request(request_id):
9751022
return redirect(url_for("profile_requests.profile_requests"))
9761023

9771024
raw_profile, errors = _parse_execution_profile_request_form()
1025+
source_profile = None
1026+
source_profile_id = profile_request.get("source_profile_id") or ""
1027+
if source_profile_id:
1028+
source_profile = next(
1029+
(
1030+
profile
1031+
for profile in store.list_profiles()
1032+
if profile["id"] == source_profile_id
1033+
),
1034+
None,
1035+
)
1036+
if not errors and source_profile and profile_request.get("request_type") in {
1037+
"change_profile",
1038+
"pause_profile",
1039+
"resume_profile",
1040+
"retire_profile",
1041+
}:
1042+
submitted_metadata = raw_profile.get("metadata_json")
1043+
submitted_metadata = submitted_metadata if isinstance(submitted_metadata, dict) else {}
1044+
raw_profile["metadata_json"] = _profile_request_followup_metadata(
1045+
source_profile,
1046+
note=str(
1047+
submitted_metadata.get("note")
1048+
or submitted_metadata.get("request_note")
1049+
or ""
1050+
),
1051+
desired_schedule=str(submitted_metadata.get("desired_schedule") or ""),
1052+
desired_watch_target=str(submitted_metadata.get("desired_watch_target") or ""),
1053+
preserve_profile_metadata=profile_request.get("request_type") == "change_profile",
1054+
)
1055+
if not errors and profile_request.get("request_type") == "change_profile":
1056+
if not source_profile:
1057+
errors.append("source profile was not found")
1058+
elif not _profile_request_has_effective_changes(raw_profile, source_profile):
1059+
errors.append(
1060+
"change request needs a schedule, watch target, or profile field change; "
1061+
"note-only changes are not actionable"
1062+
)
9781063
if errors:
9791064
ok = False
9801065
else:

result_server/templates/admin_execution_profile_requests.html

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -480,6 +480,7 @@ <h2 class="section-title">{% if review_mode %}Review Queue{% else %}My Requests{
480480
{% for item in profile_requests %}
481481
{% set profile = item.requested_profile %}
482482
{% set metadata = profile.metadata_json if profile.metadata_json is mapping else {} %}
483+
{% set request_note = metadata.request_note or metadata.note %}
483484
{% set link = profile_links.get(item.id, {}) if profile_links is mapping else {} %}
484485
{% set linked_profile = link.profile if link.profile is mapping else none %}
485486
{% set request_type_label = {
@@ -524,8 +525,8 @@ <h2 class="section-title">{% if review_mode %}Review Queue{% else %}My Requests{
524525
<span class="request-subline">watch {{ metadata.desired_watch_target }}</span>
525526
{% endif %}
526527
<span class="request-subline">{{ profile.activity or '-' }}</span>
527-
{% if metadata.note %}
528-
<span class="request-subline">{{ metadata.note }}</span>
528+
{% if request_note %}
529+
<span class="request-subline">{{ request_note }}</span>
529530
{% endif %}
530531
{% if item.request_type == 'change_profile' %}
531532
{% set requested_changes = link.requested_changes or [] %}
@@ -711,7 +712,7 @@ <h2 class="section-title">{% if review_mode %}Review Queue{% else %}My Requests{
711712
</label>
712713
<label>
713714
Note
714-
<textarea name="note" rows="2">{{ metadata.note }}</textarea>
715+
<textarea name="note" rows="2">{{ request_note }}</textarea>
715716
</label>
716717
<button type="submit" class="btn btn-primary">Revise and Resubmit</button>
717718
</form>

result_server/tests/test_execution_profiles.py

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1710,11 +1710,14 @@ def test_execution_profile_requests_keep_available_linked_profile_history(tmp_pa
17101710
def test_admin_execution_profile_requests_show_followup_target_and_note(tmp_path):
17111711
db_path = tmp_path / "cx_portal.sqlite3"
17121712
store = ExecutionProfileStore(str(db_path))
1713-
source_profile = _profile()
1713+
source_profile = _profile(metadata_json={"note": "profile note"})
17141714
store.upsert_profile(source_profile, actor="admin@test.com")
17151715
requested_profile = dict(source_profile)
17161716
requested_profile["system"] = ["SourceSystem", "PeerSystem"]
1717-
requested_profile["metadata_json"] = {"note": "change requested in note"}
1717+
requested_profile["metadata_json"] = {
1718+
"note": "profile note",
1719+
"request_note": "change requested in note",
1720+
}
17181721
store.create_profile_request(
17191722
requested_profile=requested_profile,
17201723
requester_email="applicant@test.com",
@@ -1737,9 +1740,60 @@ def test_admin_execution_profile_requests_show_followup_target_and_note(tmp_path
17371740
assert "System" in html
17381741
assert "current SourceSystem" in html
17391742
assert "requested SourceSystem, PeerSystem" in html
1740-
assert "Note" in html
1741-
assert "current -" in html
1742-
assert "requested change requested in note" in html
1743+
assert "requested change requested in note" not in html
1744+
finally:
1745+
_cleanup(temp_dirs)
1746+
1747+
1748+
def test_execution_profile_change_followup_requires_effective_change(tmp_path):
1749+
db_path = tmp_path / "cx_portal.sqlite3"
1750+
store = ExecutionProfileStore(str(db_path))
1751+
source_profile = _profile(
1752+
id="demoapp-demosystem-request",
1753+
metadata_json={"note": "profile note"},
1754+
)
1755+
store.upsert_profile(source_profile, actor="admin@test.com")
1756+
app, temp_dirs = _admin_app(db_path)
1757+
try:
1758+
with app.test_client() as client:
1759+
_login_admin(client)
1760+
note_only_resp = client.post(
1761+
"/execution-profile-requests/follow-up",
1762+
data={
1763+
"source_profile_id": "demoapp-demosystem-request",
1764+
"request_type": "change_profile",
1765+
"note": "please change this",
1766+
},
1767+
follow_redirects=True,
1768+
)
1769+
schedule_resp = client.post(
1770+
"/execution-profile-requests/follow-up",
1771+
data={
1772+
"source_profile_id": "demoapp-demosystem-request",
1773+
"request_type": "change_profile",
1774+
"desired_schedule": "15 14 * * * / Asia/Tokyo",
1775+
"note": "please change cadence",
1776+
},
1777+
follow_redirects=True,
1778+
)
1779+
review_resp = client.post(
1780+
"/admin/execution-profile-requests/1/review",
1781+
data={"action": "approve", "review_comment": "approved"},
1782+
follow_redirects=True,
1783+
)
1784+
1785+
assert note_only_resp.status_code == 200
1786+
assert b"note-only changes are not actionable" in note_only_resp.data
1787+
assert schedule_resp.status_code == 200
1788+
assert b"Execution profile follow-up request #1 submitted." in schedule_resp.data
1789+
request_row = ExecutionProfileStore(str(db_path)).get_profile_request(1)
1790+
metadata = request_row["requested_profile"]["metadata_json"]
1791+
assert metadata["note"] == "profile note"
1792+
assert metadata["request_note"] == "please change cadence"
1793+
assert review_resp.status_code == 200
1794+
result = load_execution_profiles(str(db_path))
1795+
assert result.profiles[0]["metadata_json"]["note"] == "profile note"
1796+
assert "request_note" not in result.profiles[0]["metadata_json"]
17431797
finally:
17441798
_cleanup(temp_dirs)
17451799

result_server/utils/execution_profiles.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1735,6 +1735,14 @@ def review_profile_request(
17351735
payload["status"] = "approved"
17361736
payload["approved_by"] = ""
17371737
payload["approved_at"] = ""
1738+
if request_type == "change_profile":
1739+
metadata = payload.get("metadata_json")
1740+
if isinstance(metadata, dict):
1741+
payload["metadata_json"] = {
1742+
key: value
1743+
for key, value in metadata.items()
1744+
if key != "request_note"
1745+
}
17381746
allocation_project_id = str(payload.get("allocation_project_id") or "").strip()
17391747
systems = _as_text_list(payload.get("system"))
17401748
if allocation_project_id and len(systems) != 1:

0 commit comments

Comments
 (0)