Allow independent dates while ARR field reviews are pending
This commit is contained in:
1 parent
fe8735aac2
commit
fb07580c0c
15 files changed
+316
-16
No files matched your search
@@ -7,20 +7,112 @@ const item=(extra={})=>({item_id:'1:BLOCK_CODE',source_sequence:1,confirmation_n
|
||||
const review=(items,revision=1)=>({request_id:requestId,report_date:task.report_date,status:'editing',revision,items,pending_count:items.filter(x=>!x.confirmed).length,total_count:items.length,can_finalize:items.every(x=>x.confirmed)});
|
||||
const plain=value=>JSON.parse(JSON.stringify(value));
|
||||
|
||||
test('source review opens before a job exists and choosing another date retains the original request',async()=>{
|
||||
const h=harness(()=>response(200,review([item()])));
|
||||
test('source review opens before a job exists and another date can start its own download',async()=>{
|
||||
const h=harness((url,options)=>options.method==='POST'
|
||||
? response(202,{...JSON.parse(options.body),status:'queued',job_id:null,can_retry:false})
|
||||
: response(200,review([item()])));
|
||||
await h.acceptARRDownloadTask(task,{sync:false});
|
||||
assert.equal(h.element('#arr-download-data-review').hidden,false);
|
||||
assert.equal(h.element('#arr-data-review-panel').hidden,false);
|
||||
assert.equal(h.element('#arr-download-review').hidden,true);
|
||||
assert.equal(h.element('#arr-data-review-finalize').disabled,true);
|
||||
h.element('#arr-download-date').value='2026-10-08';
|
||||
h.element('#arr-download-date').value='2026-09-17';
|
||||
h.handleARRDownloadDateChange();
|
||||
assert.equal(h.state.arrDownloadIntent.request_id,requestId,'choosing a date alone keeps the earlier task');
|
||||
assert.equal(h.element('#arr-download-button').textContent,'arr_download.start');
|
||||
assert.equal(h.element('#arr-download-button').disabled,false);
|
||||
await h.submit();
|
||||
assert.notEqual(h.state.arrDownloadIntent.request_id,requestId);
|
||||
assert.equal(h.state.arrDownloadTask.report_date,'2026-09-17');
|
||||
assert.equal(h.element('#arr-download-date').value,'2026-09-17');
|
||||
const post=h.calls.find(call=>call.method==='POST');
|
||||
assert.equal(post.url,'/api/arr-downloads');
|
||||
assert.equal(JSON.parse(post.body).report_date,'2026-09-17');
|
||||
assert.equal(h.state.arrDownloadPendingReviews[0].request_id,requestId);
|
||||
assert.equal(h.element('#arr-data-review-panel').hidden,true);
|
||||
});
|
||||
|
||||
test('submitting the pending review date only opens that review without another download',async()=>{
|
||||
const h=harness(()=>response(200,review([item()])));
|
||||
await h.acceptARRDownloadTask(task,{sync:false});
|
||||
assert.equal(h.element('#arr-download-button').textContent,'arr_download.complete_data');
|
||||
await h.submit();
|
||||
assert.equal(h.state.arrDownloadIntent.request_id,requestId);
|
||||
assert.equal(h.state.arrDownloadTask.report_date,'2026-10-07');
|
||||
assert.equal(h.element('#arr-download-date').value,'2026-10-08');
|
||||
assert.equal(h.calls.every(call=>call.method==='GET'),true,'review never creates a replacement download');
|
||||
assert.equal(h.calls.every(call=>call.method==='GET'),true);
|
||||
});
|
||||
|
||||
test('pending dates and unsaved drafts remain separate when moving between two field reviews',async()=>{
|
||||
const otherId='b'.repeat(32);
|
||||
const otherTask={...task,request_id:otherId,report_date:'2026-09-17'};
|
||||
const h=harness(url=>url.endsWith('/data-review')
|
||||
? response(200,{...review([item()]),request_id:url.includes(otherId)?otherId:requestId})
|
||||
: response(200,url.endsWith(otherId)?otherTask:task));
|
||||
await h.acceptARRDownloadTask(task,{sync:false});
|
||||
h.trackARRDataReviewDraft(h.row('1:BLOCK_CODE','OCTOBER-DRAFT').input);
|
||||
await h.acceptARRDownloadTask(otherTask,{sync:false});
|
||||
assert.equal(h.state.arrDownloadPendingReviews.length,2);
|
||||
assert.equal(h.state.arrDataReviewDrafts['1:BLOCK_CODE'],undefined);
|
||||
h.trackARRDataReviewDraft(h.row('1:BLOCK_CODE','SEPTEMBER-DRAFT').input);
|
||||
await h.selectARRPendingReview(requestId);
|
||||
assert.equal(h.state.arrDownloadTask.request_id,requestId);
|
||||
assert.equal(h.element('#arr-download-date').value,'2026-10-07');
|
||||
assert.equal(h.state.arrDataReviewDrafts['1:BLOCK_CODE'],'OCTOBER-DRAFT');
|
||||
await h.selectARRPendingReview(otherId);
|
||||
assert.equal(h.state.arrDataReviewDrafts['1:BLOCK_CODE'],'SEPTEMBER-DRAFT');
|
||||
assert.equal(h.calls.every(call=>call.method==='GET'),true,'returning to a review never re-fetches Oracle');
|
||||
});
|
||||
|
||||
test('pending dates restore from the server even when the latest task is no longer a review',async()=>{
|
||||
const h=harness(url=>url==='/api/arr-downloads'
|
||||
? response(200,{context_id:'production',ready:true,default_date:'2026-10-07',
|
||||
pending_data_reviews:[task],latest_task:{...task,request_id:'b'.repeat(32),report_date:'2026-09-17',status:'failed'}})
|
||||
: response(200,url.endsWith('/data-review')?review([item()]):task));
|
||||
await h.initARRDownload();
|
||||
assert.equal(h.state.arrDownloadTask,null);
|
||||
assert.equal(h.element('#arr-download-pending').hidden,false);
|
||||
assert.match(h.element('#arr-download-pending-list').innerHTML,/2026-10-07/);
|
||||
await h.selectARRPendingReview(requestId);
|
||||
assert.equal(h.state.arrDownloadTask.request_id,requestId);
|
||||
assert.equal(h.state.arrDataReview.pending_count,1);
|
||||
});
|
||||
|
||||
test('an uncertain new date cannot be abandoned by opening an earlier field review',async()=>{
|
||||
const h=harness(()=>{throw new Error('unexpected request');});
|
||||
h.state.arrDownloadPendingReviews=[task];
|
||||
h.rememberARRIntent({request_id:'b'.repeat(32),report_date:'2026-09-17'});
|
||||
h.state.arrDownloadDisconnected=true;
|
||||
await h.selectARRPendingReview(requestId);
|
||||
assert.equal(h.calls.length,0);
|
||||
assert.equal(h.state.arrDownloadIntent.report_date,'2026-09-17');
|
||||
});
|
||||
|
||||
test('a completed pending review can still be finalized after reopening it',async()=>{
|
||||
const h=harness(url=>response(200,url.endsWith('/data-review')
|
||||
? review([item({confirmed:true,value:'VERIFIED'})]) : task));
|
||||
h.state.arrDownloadPendingReviews=[task];
|
||||
await h.selectARRPendingReview(requestId);
|
||||
assert.equal(h.state.arrDownloadBusy,false);
|
||||
assert.equal(h.arrDataReviewCanFinalize(),true);
|
||||
assert.equal(h.element('#arr-data-review-finalize').disabled,false);
|
||||
});
|
||||
|
||||
test('lost submission for another date retains both its uncertain intent and the earlier pending review',async()=>{
|
||||
const h=harness((url,options)=>{
|
||||
if(options.method==='POST') throw new TypeError('lost response');
|
||||
if(url.endsWith('/data-review')) return response(200,review([item()]));
|
||||
return response(404,null,'ARR_DOWNLOAD_NOT_FOUND');
|
||||
});
|
||||
await h.acceptARRDownloadTask(task,{sync:false});
|
||||
h.element('#arr-download-date').value='2026-09-17';
|
||||
h.handleARRDownloadDateChange();
|
||||
await h.submit();
|
||||
const intent=plain(h.state.arrDownloadIntent);
|
||||
assert.equal(intent.report_date,'2026-09-17');
|
||||
assert.notEqual(intent.request_id,requestId);
|
||||
assert.equal(h.state.arrDownloadTask.status,'not_received');
|
||||
assert.equal(h.state.arrDownloadPendingReviews[0].request_id,requestId);
|
||||
await h.selectARRPendingReview(requestId);
|
||||
assert.deepEqual(plain(h.state.arrDownloadIntent),intent);
|
||||
});
|
||||
|
||||
test('incomplete review cannot finalize even when server flag says it can',async()=>{
|
||||
|
||||
@@ -22,7 +22,7 @@ function harness(respond) {
|
||||
localStorage:{setItem:(k,v)=>storage.set(k,v),getItem:k=>{storageReads.push(k);return storage.get(k);},removeItem:k=>storage.delete(k)},
|
||||
fetch:async (url, options) => {calls.push({url,method:options.method||'GET',body:options.body});return respond(url,options,calls);},
|
||||
});
|
||||
const exports='state, submitARRDownload, rememberARRIntent, acceptARRDownloadTask, handleARRDownloadDateChange, initARRDownload, loadARRDownloadTask, loadARRDataReview, renderARRDataReview, arrDataReviewCanFinalize, saveARRDataReviewItem, finalizeARRDataReview, trackARRDataReviewDraft, parseARRDataReviewValue';
|
||||
const exports='state, submitARRDownload, rememberARRIntent, acceptARRDownloadTask, handleARRDownloadDateChange, initARRDownload, loadARRDownloadTask, loadARRDataReview, renderARRDataReview, arrDataReviewCanFinalize, saveARRDataReviewItem, finalizeARRDataReview, trackARRDataReviewDraft, parseARRDataReviewValue, selectARRPendingReview';
|
||||
vm.runInContext(source.replace(boot,` globalThis.subject = {${exports}};\n})();`),context);
|
||||
const subject=context.subject;
|
||||
Object.assign(subject.state,{arrDownloadReady:true,arrDownloadLoaded:true,arrDownloadStorageKey:'test',arrDownloadUsername:'operator',csrf:'fixture'});
|
||||
|
||||
@@ -74,6 +74,19 @@ class LocalOHIPTests(unittest.TestCase):
|
||||
with self.assertRaises(PortalError): local.AuthorizedExecutor(executor, self.access).execute(request_id="a" * 32)
|
||||
executor.execute.assert_not_called()
|
||||
|
||||
def test_pending_source_reviews_are_readable_without_new_access_or_acquisition(self):
|
||||
pending = [{"request_id": "a" * 32, "report_date": "2026-10-07", "status": "needs_data_review"}]
|
||||
queue = Mock(ready=True)
|
||||
queue.pending_data_reviews.return_value = pending
|
||||
access = Mock()
|
||||
access.status.return_value = {"ready": False}
|
||||
downloads = local.PermissionDownloads(queue, access)
|
||||
self.assertFalse(downloads.ready)
|
||||
self.assertEqual(downloads.pending_data_reviews(), pending)
|
||||
queue.pending_data_reviews.assert_called_once_with()
|
||||
access.require_ready.assert_not_called()
|
||||
queue.create.assert_not_called()
|
||||
|
||||
def test_source_policy_identity_cannot_be_rebound_to_demo_or_another_hotel(self):
|
||||
config = document(self.root / "instance.json")
|
||||
config["hotel_id"] = "OTHER"
|
||||
|
||||
@@ -9,7 +9,7 @@ import tempfile
|
||||
import time
|
||||
import unittest
|
||||
import sys
|
||||
from unittest.mock import patch
|
||||
from unittest.mock import Mock, patch
|
||||
|
||||
from arr_ingestion.repository import InMemoryIngestionRepository
|
||||
from arr_ingestion.service import IngestionService
|
||||
@@ -22,7 +22,7 @@ from arr_web.auth import LoginCredentials
|
||||
from arr_web.local_replay import create_instance, open_instance
|
||||
from arr_web.local_replay_database import ReplayDatabase
|
||||
from arr_web.local_xml_replay import (COOKIE, LocalReplayPortal, NativeXMLSnapshot,
|
||||
NativeXMLReplayExecutor, document)
|
||||
NativeXMLReplayExecutor, SourceDateDownloads, document)
|
||||
from tests.test_arr_opera_daily_ingest import reservation, xml_document
|
||||
from tests.test_ohip_processing_handoff import CountingProcessor
|
||||
from integrations.ohip.collect_arr_source import CollectionError
|
||||
@@ -87,6 +87,15 @@ class NativeReplayTests(unittest.TestCase):
|
||||
NativeXMLSnapshot.create(self.root / "bad", self.source, pin, day)
|
||||
self.assertFalse((self.root / "bad").exists())
|
||||
|
||||
def test_source_date_wrapper_preserves_pending_review_list(self):
|
||||
pending = [{"request_id": REQUEST, "report_date": DAY.isoformat(), "status": "needs_data_review"}]
|
||||
queue = Mock()
|
||||
queue.pending_data_reviews.return_value = pending
|
||||
downloads = SourceDateDownloads(queue, self.snapshot)
|
||||
self.assertEqual(downloads.pending_data_reviews(), pending)
|
||||
queue.pending_data_reviews.assert_called_once_with()
|
||||
queue.create.assert_not_called()
|
||||
|
||||
def test_source_tampering_stops_before_processing_or_database(self):
|
||||
(self.root / "fixture/source.xml").write_bytes(self.raw + b" ")
|
||||
with self.assertRaises(ValueError):
|
||||
|
||||
@@ -110,6 +110,50 @@ class DownloadTests(unittest.TestCase):
|
||||
self.assertEqual(self.service.retry("a" * 32), result)
|
||||
self.assertEqual(len(self.executor.calls), 1)
|
||||
|
||||
def test_pending_source_reviews_coexist_for_different_dates_and_coalesce_per_day(self):
|
||||
self.executor.outcome = DownloadOutcome("needs_data_review")
|
||||
self.executor.release.set()
|
||||
self.service.create("2026-10-07", "a" * 32)
|
||||
october = self.finished()
|
||||
self.service.create("2026-09-17", "b" * 32)
|
||||
september = self.finished("b" * 32)
|
||||
self.assertEqual(october["status"], "needs_data_review")
|
||||
self.assertEqual(september["status"], "needs_data_review")
|
||||
self.assertEqual(self.service.pending_data_reviews(), [october, september])
|
||||
self.assertEqual(self.service.latest(), september)
|
||||
self.assertEqual(self.service.create("2026-10-07", "c" * 32), october)
|
||||
self.assertEqual(self.service.create("2026-09-17", "d" * 32), september)
|
||||
self.assertEqual([call["from_date"] for call in self.executor.calls],
|
||||
[date(2026, 10, 7), date(2026, 9, 17)])
|
||||
|
||||
def test_pending_source_review_list_survives_restart_without_reexecuting(self):
|
||||
self.executor.outcome = DownloadOutcome("needs_data_review")
|
||||
self.executor.release.set()
|
||||
self.service.create("2026-09-17", "a" * 32)
|
||||
self.finished()
|
||||
self.service.create("2026-10-07", "b" * 32)
|
||||
self.finished("b" * 32)
|
||||
expected = self.service.pending_data_reviews()
|
||||
self.service.close(wait=True)
|
||||
self.service = PersistentARRDownloads(self.root, self.executor)
|
||||
self.assertEqual(self.service.pending_data_reviews(), expected)
|
||||
self.assertEqual(self.service.pending_data_reviews(), expected)
|
||||
self.assertEqual([task["report_date"] for task in expected], ["2026-10-07", "2026-09-17"])
|
||||
self.assertEqual(len(self.executor.calls), 2)
|
||||
|
||||
def test_pending_source_review_list_excludes_other_outcomes(self):
|
||||
self.assertEqual(self.service.pending_data_reviews(), [])
|
||||
self.executor.release.set()
|
||||
self.service.create("2026-09-17", "a" * 32)
|
||||
self.finished()
|
||||
self.executor.outcome = DownloadOutcome("needs_review", "arrjob-price-review")
|
||||
self.service.create("2026-09-18", "b" * 32)
|
||||
self.finished("b" * 32)
|
||||
self.executor.outcome = DownloadOutcome("failed")
|
||||
self.service.create("2026-09-19", "c" * 32)
|
||||
self.finished("c" * 32)
|
||||
self.assertEqual(self.service.pending_data_reviews(), [])
|
||||
|
||||
def test_capture_only_success_without_processing_job_is_not_accepted(self):
|
||||
self.executor.outcome = DownloadOutcome("succeeded")
|
||||
self.executor.release.set()
|
||||
@@ -226,10 +270,39 @@ class DownloadRoutesTests(unittest.TestCase):
|
||||
config = json.loads(app.handle("GET", "/api/arr-downloads", headers).body)["data"]
|
||||
self.assertFalse(config["ready"])
|
||||
self.assertIsNone(config["latest_task"])
|
||||
self.assertEqual(config["pending_data_reviews"], [])
|
||||
response = app.handle("POST", "/api/arr-downloads", headers,
|
||||
json.dumps({"report_date": "2026-09-15", "request_id": "a" * 32}).encode())
|
||||
self.assertEqual(response.status, 503)
|
||||
|
||||
def test_configuration_lists_pending_dates_without_changing_latest_task(self):
|
||||
self.executor.outcome = DownloadOutcome("needs_data_review")
|
||||
self.executor.release.set()
|
||||
for day, request_id in (("2026-10-07", "a" * 32), ("2026-09-17", "b" * 32)):
|
||||
self.assertEqual(self.post({"report_date": day, "request_id": request_id}).status, 202)
|
||||
deadline = time.monotonic() + 4
|
||||
while time.monotonic() < deadline:
|
||||
if self.service.get(request_id)["status"] == "needs_data_review":
|
||||
break
|
||||
time.sleep(.01)
|
||||
else:
|
||||
self.fail("worker did not reach source review")
|
||||
snapshot = self.app._arr_download_snapshot
|
||||
with patch.object(self.app, "_arr_download_snapshot", wraps=snapshot) as snapshot_call:
|
||||
response = self.app.handle("GET", "/api/arr-downloads", self.headers)
|
||||
self.assertEqual(response.status, 200)
|
||||
config = json.loads(response.body)["data"]
|
||||
self.assertEqual(config["latest_task"]["request_id"], "b" * 32)
|
||||
self.assertEqual([task["report_date"] for task in config["pending_data_reviews"]],
|
||||
["2026-10-07", "2026-09-17"])
|
||||
self.assertEqual(config["pending_data_reviews"], self.service.pending_data_reviews())
|
||||
self.assertEqual(snapshot_call.call_count, 3)
|
||||
self.assertEqual(self.app.handle("GET", "/api/arr-downloads", {}).status, 401)
|
||||
for task in config["pending_data_reviews"]:
|
||||
self.assertIsNone(task["job_id"])
|
||||
self.assertNotIn("attempts", task)
|
||||
self.assertNotIn("records", task)
|
||||
|
||||
def test_default_day_uses_bangkok_calendar_at_year_boundary(self):
|
||||
with patch("arr_web.arr_downloads.datetime") as clock:
|
||||
clock.now.return_value = datetime(2026, 1, 1, 0, 1, tzinfo=ZoneInfo("Asia/Bangkok"))
|
||||
|
||||
Reference in new issue
Block a user