Rescue master branch commits

[maxime-rainville]

There's still some potentially useful work in the master branches. Those were meant to be to bu sue for CMS 5, however they are horribly out of date. There might be some useful commits there that we want to fold back into the new major release branches.

Acceptance criteria

• Commits that are unique to the master branches and are useful are cherry-picked tot he new major branches.
• Commits don't make sense with work that got completed after they got merged into master get left behind.
• Commits that wold introduce major upgrade pains are left behind.
• Breaking changes are noted in the appropriate documentation
• Create follow task to delete old master branches to give people a chance to rescue any commits they care about.

Depends on #10349

Notes

• There was a spike for this which may be useful: https://github.com/silverstripeltd/product-issues/issues/539

PRs

• API rescue master-branch PR: Use Generators for ORM #10450
  • API Rescue Master Branch PR: Update PostgreSQLQuery to use generators silverstripe-postgresql#135
• API rescue master-branch PR: Shorten auto-generated table names #7621 #10454
• API Rescue Master Branch PR: Remove isDev / isTest querystring arguments #10456
• API Rescue Master Branch PR: Only expose public extension methods #10457
  • API Use strong types from silverstripe/framework#10457 silverstripe-cms#2764
  • API Rescue Master Branch PR: Support new extend*() format for extension methods silverstripe-versioned#371
• API Rescue Master Branch PR: Update MigrationTask class to be an abstract class #10458
• ENH Rescue Master Branch Commits: CSV BOM stripping is now handled internally by league/csv #10459
• API Rescue Master Branch PR: Allow dataobject get_one without passing a class #10460
• API Rescue Master Branch PR: TestCase classes should be abstract to prevent warnings #10461
• Rescue Master Branch PR: Fix SQLConditionalExpression::getJoins so it always adds explicit aliases #10462
• Rescue Master Branch PR: ENH Various changes via static analysis tooling #10463
• FIX Use publishRecursive() over doPublish() silverstripe-cms#2765
• Rescue Master Branch PR: MNT Use SiteTree instead of Page in tests silverstripe-cms#2766
• Rescue Master Branch PR: FIX Don't offer to restore to root if can_be_root is false silverstripe-cms#2768
• API Rescue Master Branch PR: Remove deprecated CMSPageHistoryViewer silverstripe-cms#2769
• Rescue Master Branch PR: ENH Remove legacy translation behaviour in template silverstripe-admin#1346
• BUG case error in SelectionGroup template silverstripe-admin#1349
• Rescue Master Branch PR: ENH Stop using old deprecated allVersions call silverstripe-assets#507
• DOC Fix doc for FileNameFilter silverstripe-assets#508
• Rescue Master Branch commit: FIX Replace references to deprecated allVersions() silverstripe-versioned-admin#249
• Rescue Master Branch Commit ENH Prefer ::class syntax over class strings silverstripe-versioned#372
• Rescue Master Branch PR: PSR12 compliance silverstripe-reports#154
• API Rescue Master Branch Commit: Remove ElementalSolrIndex silverstripe-elemental#1002
• DOC Document changes in various rescued master commits developer-docs#70
• API rescue master-branch PR: Use Generators for ORM #10484 (again)
  • API Update PostgreSQLQuery to use generators silverstripe-postgresql#138 (again)
  • FIX Reset query generator after iteration is complete silverstripe-postgresql#139
• Rescue master branch PRs: Template enhancements #10497
• ENH Add PHP 8.1 safe null-coalescing operators to peg file #10507
• DOC Document breaking changes to template syntax developer-docs#81

PRs for broken builds

• ENH Don't call deprecated constructExtensions method silverstripe-asset-admin#1290
• ENH Stop using deprecated Versioned methods silverstripe-frameworktest#121

[GuySartorelli]

silverstripe/silverstripe-sqlite3#44 will need to be rescued once a new major release branch is created for that module.

[kinglozzer]

#8152 and #8456 seem relatively low risk to me - the only BC issues I can think of are:

• Someone passing true, false, null or a number and expecting strings rather than scalars/null. I think it’s unlikely, and PHP’s loose typing makes it less likely to be an issue
• Someone typehinting strings for method args

Both cases would be highlighted pretty clearly by errors when the templates run, and would be easy to fix. If anyone needs to preserve the current behaviour, they can quote any values they’ve got hardcoded in templates (it only applies to $Foo(123), not $Foo($SomethingReturningAnInteger))

#8148 is the more controversial one because it likely won’t error - it’ll either stop outputting data, or output data from the wrong “scope”. The problem with that one is that there’s no way to fix #4015 without causing that upgrade pain - so either we suck it up at some point or never fix it 😕

[emteknetnz]

I'm happy to have all three merged.

Applications currently passing false or null from a template would be currently working by accident because there is broken logic in the controller i.e. if ($myTemplateVar) ... - string "false" is truthy. So the fact it breaks on upgrade is actually what's supposed to happen.

For the $Up.Title, it's very easy to provide clear upgrade instructions here, simply do a project string find and replace for possible template strings such $Up. (with a leading space) or ($Up. in template files.

[emteknetnz]

All linked PRs have been merged

[GuySartorelli]

Reopening - there is still the discussion about the potentially controversial PRs which have not yet been rescued.

[maxime-rainville]

I also don't think any of those changes are likely to cause a great deal of upgrade headaches. I'm sure it will trip somebody somewhere, but that seems reasonable.

On #8148, it's not clear to me the new behaviour is obviously better than the old one. I don't really have strong feeling either way. Looking at the matching #4015 issue, I see a mix of view there as well.

My instinct would be to

• recover
  • API: Preserve variable types in template lookup args (closes #4822) #8152
  • Add support for null values in template lookups #8456
• let API: <% loop %> and <% with %> only ever create one new scope level (fixes #4015) #8148 go

[GuySartorelli]

I agree with Steve that it's pretty easy to call out how to upgrade the scope change (and for a major release people are much more likely to actually look at the changelog to see what they may need to change), and the majority consensus seems to be that it's a worthwhile change - I'll rescue all three PRs.

[GuySartorelli]

Blocked pending alpha1 being tagged.