DistributedProofreaders / dproofreaders

Distributed Proofreaders is a web application intended to ease the process of converting public domain books into e-texts.
https://www.pgdp.net
GNU General Public License v2.0
46 stars 28 forks source link

Add many property declarations to classes so PHPStan can type check, part 2 #1189

Closed bpfoley closed 1 month ago

bpfoley commented 2 months ago

Cherry picking the remainder of the class property declarations from #1112 so they can be submitted separately.

There were lots of merge conflicts doing this, so it turned out to be easier to cherry pick everything into a new PR. I'll close #1112.

Sandbox: https://www.pgdp.org/~cpeel/c.branch/property-2/ (changed to cpeel's sandbox to pick up a small fix)

cpeel commented 2 months ago

Testing advice

This is a behind-the-scenes code robustness PR that should be completely invisible to end users. The most likely failure scenario is there to be a PHP error error, warning, or notice show up somewhere unexpected. The behavior of this sandbox should match the main TEST server (in most ways, outside of code in pgdp-production).

Areas of focus

The code changes are primarily focused in these areas:

cpeel commented 2 months ago

Added a fixup commit to address a problem when editing a non-posted project. This is why it had the !empty() check 😁 . I tested that the Posted to PG transition detour works with this change too.

@bpfoley at your convenience if you could update your sandbox that would be appreciated.

theshriek commented 2 months ago

I tried to create a new project and when I clicked on "No Matches", I got this message.

Fatal error: Uncaught Error: Typed property ProjectInfoHolder::$posted must not be accessed before initialization in /home/bfoley/public_html/c.branch/property-2/tools/project_manager/editproject.php:456 Stack trace: #0 /home/bfoley/public_html/c.branch/property-2/tools/project_manager/editproject.php(425): ProjectInfoHolder->show_hidden_controls() #1 /home/bfoley/public_html/c.branch/property-2/tools/project_manager/editproject.php(120): ProjectInfoHolder->show_form() #2 {main} thrown in /home/bfoley/public_html/c.branch/property-2/tools/project_manager/editproject.php on line 456

cpeel commented 2 months ago

I tried to create a new project and when I clicked on "No Matches", I got this message.

@theshriek can you please try your testing again in the https://www.pgdp.org/~cpeel/c.branch/property-2/ sandbox? I pushed a fix to the branch and Brian is currently out of pocket to update his so I created my own with the fix.

theshriek commented 2 months ago

@cpeel that worked. Thanks!

bpfoley commented 2 months ago

Thanks @theshriek for testing, and @cpeel for the fix.

I've been mostly offline for a few days. I've rolled the fixup into an updated patchset and pushed it. The sandbox at https://www.pgdp.org/~cpeel/c.branch/property-2/ also has the fix.

ellinora commented 2 months ago

In "Your ## Neighborhood" section in right panel for P1, P2, F2, I get an error message (P3 and F1 look OK). https://www.pgdp.org/~cpeel/c.branch/property-2/stats/pages_proofed_graphs.php?tally_name=F2 (through Stats Central) https://www.pgdp.org/~cpeel/c.branch/property-2/tools/proofers/round.php?round_id=F2 (through F2 in menu bar)

Fatal error: Uncaught TypeError: Typed property PageTally_Neighbor::$username must be string, null used in /home/cpeel/public_html/c.branch/property-2/pinc/page_tally.inc:184 Stack trace: #0 /home/cpeel/public_html/c.branch/property-2/pinc/page_tally.inc(165): PageTally_Neighbor->__construct() #1 /home/cpeel/public_html/c.branch/property-2/pinc/theme.inc(794): user_get_page_tally_neighborhood() #2 /home/cpeel/public_html/c.branch/property-2/pinc/theme.inc(571): show_tally_specific_stats() #3 /home/cpeel/public_html/c.branch/property-2/pinc/theme.inc(80): html_statsbar() #4 [internal function]: output_footer() #5 {main} thrown in /home/cpeel/public_html/c.branch/property-2/pinc/page_tally.inc on line 184

cpeel commented 2 months ago

In "Your ## Neighborhood" section in right panel for P1, P2, F2, I get an error message (P3 and F1 look OK). https://www.pgdp.org/~cpeel/c.branch/property-2/stats/pages_proofed_graphs.php?tally_name=F2 (through Stats Central) https://www.pgdp.org/~cpeel/c.branch/property-2/tools/proofers/round.php?round_id=F2 (through F2 in menu bar)

Oh good catch! If someone has requested anonymous stats, the username and date_joined for that user will be null in in the neighbor sidebar.

I've pushed up a fixup commit to the branch and updated my copy of the sandbox.

ellinora commented 2 months ago

Great, now I'm seeing the neighborhoods for all the rounds, the error message is gone.