Skip to content

(#4815) Fix PDQ phpstan issues#4923

Merged
blairlearn merged 1 commit intodevelopfrom
ticket/4815-phpstan-issues
Mar 18, 2026
Merged

(#4815) Fix PDQ phpstan issues#4923
blairlearn merged 1 commit intodevelopfrom
ticket/4815-phpstan-issues

Conversation

@bkline
Copy link
Copy Markdown
Contributor

@bkline bkline commented Aug 22, 2025

In pdq_cancer_information_summary/src/OrphanCleanup.php:

  • remove line of code inadvertently copied from another method
  • reassure PHP (and phpstan) that we have the the right interface for invoking deleteRevision()

Closes #4815

@bkline bkline requested a review from blairlearn August 22, 2025 16:02
@bkline bkline requested a review from a team as a code owner August 22, 2025 16:02
@bkline bkline requested a review from bryanpizzillo August 22, 2025 16:09
@blairlearn blairlearn force-pushed the ticket/4815-phpstan-issues branch 2 times, most recently from f868416 to fa1dfed Compare September 18, 2025 21:03
@bryanpizzillo
Copy link
Copy Markdown
Member

@blairlearn what is the status of this?

@blairlearn
Copy link
Copy Markdown
Contributor

From Teams conversation

Blair
The "phpstan for PDQ" PR -- #4923

The change looks good to me, and I've done some simple testing.
But against the bigger picture of the CDR going away, that's been the extent of it.

(i.e. I've not asked Bob to send edited summaries.)

Bryan
I think we are actually going to merge this during the feature branch work. Basically it should get tested with Summary transforms...

@welshja
Copy link
Copy Markdown
Collaborator

welshja commented Nov 18, 2025

To test, we'll need to get this onto an ODE and then run the new loader to confirm that things work as expected and all summaries are loaded with no crazy errors/fires/natural disasters. (Orphaned cleanup part of the loading process runs without issue.)

@jfrank-nih jfrank-nih force-pushed the ticket/4815-phpstan-issues branch from fa1dfed to ec192c8 Compare November 19, 2025 16:56
@cgdp-management-server
Copy link
Copy Markdown

cgdp-management-server Bot commented Nov 19, 2025

ODE Deployment

Code has been deployed to ODE 1091.

@welshja
Copy link
Copy Markdown
Collaborator

welshja commented Nov 26, 2025

@jfrank-nih to check with @blairlearn in terms of any testing steps to review on the ODE where it is currently deployed.

@welshja welshja requested review from jfrank-nih and removed request for bryanpizzillo December 11, 2025 18:08
In pdq_cancer_information_summary/src/OrphanCleanup.php:
 - remove line of code inadvertently copied from another method
 - reassure PHP (and phpstan) that we have the the right interface
   for invoking deleteRevision()

Closes #4815
@blairlearn blairlearn force-pushed the ticket/4815-phpstan-issues branch from ec192c8 to 07d54f5 Compare March 16, 2026 18:25
@welshja
Copy link
Copy Markdown
Collaborator

welshja commented Mar 17, 2026

Tested on ODE1091 with CIS/DIS loader workflows. Cleanup still working as expected.

To test:
Cancer Info Summaries:

  1. Gave site admin login PDQ Importer permissions on ODE with this code installed on it.
  2. Ran CIS loader for on ODE with this change for two summaries (62915 256720)
  3. Created 22 revisions for 62915 and 3 for the 256720
  4. Re-ran loader
  5. Checked that only last 3 (current + 2 previous) revisions exist and older ones were cleaned up

Drug Info Summaries:

  1. Gave site admin login PDQ Importer permissions on ODE with this code installed on it.
  2. Ran CIS loader for on ODE with this change for two summaries (813502 817456)
  3. Created 11 revisions for 813502 and 5 for the 817456
  4. Re-ran loader
  5. Checked that only last 3 (current + 2 previous) revisions exist

@welshja welshja self-requested a review March 17, 2026 21:03
Copy link
Copy Markdown
Collaborator

@welshja welshja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good to me after testing

@blairlearn blairlearn merged commit 2a33824 into develop Mar 18, 2026
5 checks passed
@blairlearn blairlearn deleted the ticket/4815-phpstan-issues branch March 18, 2026 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PHPStanCleanup of Deprecated Function and Logic Error Noticed

5 participants