T3348-Fix-failing-quality_test_Global_Childpool

[loris-fab]

Goal

Remove the "365 children" button of the Global Childpool search, and all the code behind it.

The button was originally reported as hanging indefinitely, and this branch first fixed it: the 365 daily requests were parallelized (~10 s instead of ~1m30), a pagination bug was corrected and the GMC error handling was made explicit. In the meantime, SDS confirmed that this feature, added for one specific request, reserving children with birthdays covering the whole year, has almost no chance of ever being reused. So instead of maintaining it, we drop it: the goal of this PR is now the removal, which also lightens the code.

Technical aspect

• child_compassion/views/global_childpool_view.xml: removed the do_365_mix button and the red "Missing birthdates" block.
• child_compassion/wizards/global_child_search.py: removed the do_365_mix() method, the missing_dates field and the now unused relativedelta import.
• The earlier work on this branch (parallelization, error handling, OnrampConnector hardening) is reverted: it only existed to make this search usable.
• Checked that nothing else references do_365_mix / missing_dates (no other view, wizard or addon repo depends on them).

Misc

• The missing_dates column stays in the DB until the module is upgraded; compassion.childpool.search is a TransientModel, so there is no data to migrate.

[Copilot]

Pull request overview

This PR addresses a perceived “infinite loading” issue on the Global Childpool “365 children” action by parallelizing daily GMC API calls and improving UX with a confirmation dialog, while also fixing correctness issues in the prior sequential logic.

Changes:

• Reworked do_365_mix() to prepare per-day requests and execute them concurrently via ThreadPoolExecutor, then re-sort results chronologically.
• Fixed correctness issues around pagination (skip) and the nb_found counter by resetting skip and summing NumberOfBeneficiaries.
• Added a confirmation popup on the “365 children” button to set user expectations before starting the long-running search.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
child_compassion/wizards/global_child_search.py	Parallelizes the 365/366-day GMC search and adjusts result aggregation/state updates.
child_compassion/views/global_childpool_view.xml	Adds a user confirmation prompt before launching the “365 children” search.

Suppressed comments (3)

child_compassion/wizards/global_child_search.py:368

• future.result() will raise if any HTTP call errors, which will abort the entire 365-day search and leave the wizard in a partially-reset state. It’s safer to handle exceptions per-future and record the date as missing (or surface a summarized error) rather than crashing the whole run.

        with concurrent.futures.ThreadPoolExecutor(max_workers=15) as executor:
            futures = [executor.submit(fetch_http, req) for req in prepared_requests]
            for future in concurrent.futures.as_completed(futures):
                results.append(future.result())

child_compassion/wizards/global_child_search.py:380

• OnrampConnector.send_message() sets result["content"] to a string when the response isn’t valid JSON. In that case, result.get("content", {}).get(...) will raise because strings don’t have .get(), breaking the whole loop. Guard the type before treating content as a dict.

        for c_date, result in results:
            if result.get("code") == 200 and result.get("content", {}).get(result_name):
                total_matching_found += result["content"].get(
                    "NumberOfBeneficiaries", 0
                )
                children_data = result["content"][result_name]
                for child_data in children_data:

child_compassion/wizards/global_child_search.py:317

• missing_dates is only written at the very end of the method. If an exception occurs before the final write() (for example during concurrent requests), the wizard can keep stale missing_dates from a previous run while already having deleted global_child_ids. Reset it up-front to keep the wizard state consistent even on failure.

        self.global_child_ids.unlink()

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[greptile-apps]

Confidence Score: 4/5

Not safe to merge until the Connect retry path preserves or refreshes OAuth authentication.

The reproduced failure affects recovery from transient Connect network errors: the first failed request replaces the shared authenticated session, while the retry and later requests no longer carry the bearer token.

Files Needing Attention: message_center_compassion/tools/onramp_connector.py

T-Rex Logs

What T-Rex did

• T-Rex produced and posted a proof for a P1 finding, including a focused executable retry authentication harness.
• T-Rex validated the P1 finding by examining the harness outputs and related logs, confirming the direct-success OAuth preservation and the OAuth-free retry path.
• T-Rex posted a second P1 finding proof.
• T-Rex completed contract validation, collecting and saving baseline, retry, and harness-source artifacts for upload.

_{Ran code and verified through T-Rex}

Comments Outside Diff (2)

1. message_center_compassion/tools/onramp_connector.py, line 127-129 (link)

   Connection retry drops OAuth authentication

   When a request raises ConnectionError, this creates a replacement session but restores only its query parameters. The OAuth Authorization header installed by _retrieve_token() is discarded, so the retry and subsequent calls made through the shared client can be rejected as unauthenticated. Preserve the authenticated headers when replacing the session, or obtain a token for the new session before retrying.

   Artifacts

   
   

   Focused executable retry authentication harness

   • The authored Python harness stubs Odoo and requests, authenticates the initial session, and runs both no-retry and retry paths; it directly exercises the affected code path.

   Captured source of the focused retry authentication harness

   • Command output captures the exact executable test source used for the reproduction, ending with a self-contained record of the test.

   Direct-success request preserves OAuth authorization

   • A successful first POST uses `Bearer original-token`, creates one session, and returns HTTP 200; this is the authenticated baseline.

   Connection-error retry succeeds without OAuth authorization

   • After the simulated first connection failure, the retry returns HTTP 200 with `retry_authorization=None` while API parameters persist; this verifies authentication is lost.

   

   _{Ran code and verified through T-Rex}

2. General comment

   Connection retry loses OAuth authentication

   • Bug
     • VERIFIED at message_center_compassion/tools/onramp_connector.py:125-129. With an initially authenticated fake session, the first POST raised ConnError and the second POST succeeded, but its effective Authorization header was None. The original session sent Bearer original-token; the replacement retained only {'api_key': 'api-key', 'gpid': 'gpid'}.
   • Cause
     • The exception handler stores self._session.params, creates a new requests.Session(), and restores only .params. It neither preserves session headers nor retrieves a token for the new session.
   • Fix
     • When replacing the session, preserve/copy the authenticated headers as well as params, or explicitly invoke token retrieval for the replacement session before retrying.

   _{Ran code and verified through T-Rex}

_{Reviews (6): Last reviewed commit: "[T3348] REFACTOR: remove the 365 childre..." | Re-trigger Greptile}