Release 7.3.0 - Update to php 8.3

[jason-jackson]

ITSE-1300

---

Changed

• Update to php 8.3
• Switch adldap2/adldap2 to directorytree/ldaprecord

---

PR Checklist

• Put version number in PR title (e.g. Release x.y.z - Summary of changes)
• Documentation (README, etc.)
• Unit tests created or updated
• Run make composershow

[briskt]

Good to see that no code changes were needed.

[forevermatt]

If you would, please update composer dependencies along with this PHP version upgrade

[mtompset]

Do we even used LDAP at all? Rather than fixing the code, I think removing it might be cleaner. Less is more.

[mtompset]

I'd prefer recommended tiny clean ups before approving, though this code is functional (at least from eye-balling it).

[briskt]

Do we even used LDAP at all? Rather than fixing the code, I think removing it might be cleaner. Less is more.

Unfortunately, yes. I was about to say we use it, but remembered that Wycliffe doesn't use it any longer and they were the only ones that did. Rather than remove it in this PR, I would recommend removing it first and then revisit this upgrade. But I guess you can proceed with this change and then rip it out.

[briskt]

Since we can't test that this new LDAP implementation works, I don't like the idea of pushing out a release with it.

[jason-jackson]

Do we even used LDAP at all? Rather than fixing the code, I think removing it might be cleaner. Less is more.

Unfortunately, yes. I was about to say we use it, but remembered that Wycliffe doesn't use it any longer and they were the only ones that did. Rather than remove it in this PR, I would recommend removing it first and then revisit this upgrade. But I guess you can proceed with this change and then rip it out.

Yeah, I don't like doing both. Other option would be to upgrade PHP without any dependencies updated (I think everything still works) and then remove this.

[briskt]

Do we even used LDAP at all? Rather than fixing the code, I think removing it might be cleaner. Less is more.

Unfortunately, yes. I was about to say we use it, but remembered that Wycliffe doesn't use it any longer and they were the only ones that did. Rather than remove it in this PR, I would recommend removing it first and then revisit this upgrade. But I guess you can proceed with this change and then rip it out.

Yeah, I don't like doing both. Other option would be to upgrade PHP without any dependencies updated (I think everything still works) and then remove this.

There were a lot of dependency updates, so it's kind of a gamble what will or will not work on 8.3

[mtompset]

If there are models involved, the base models may not be generating well since Yii 2.0.52. Check the scoping and data typing in the base models on constants. If it is lacking, check out MailAdmin's gii folder, and console.php in the config directory. Then generate with "psr-12" like in scripts' tables.sh, which is triggered by make baseModels.

[forevermatt]

@jason-jackson Is this PR still in progress? I see that we have a separate 7.2.0 release already (so this might be 7.3.0 now), but those changes haven't been pulled into this feature branch yet.

I wonder if this should be set aside until after we finish removing LDAP support (if we can / are going to do that). Or maybe that's already your plan?

If needed, you could also do a separate dependency update first, then do the PHP-8.3-upgrade-with-another-dependency-update, to help keep each set of changes smaller. Just an idea, though I don't know if that would help.

[briskt]

Thanks for patiently working through this.

[briskt]

I see that we have a separate 7.2.0 release already (so this might be 7.3.0 now)

I edited the PR title

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
3.6% Duplication on New Code

See analysis details on SonarQube Cloud

[jason-jackson]

@jason-jackson Is this PR still in progress? I see that we have a separate 7.2.0 release already (so this might be 7.3.0 now), but those changes haven't been pulled into this feature branch yet.

I wonder if this should be set aside until after we finish removing LDAP support (if we can / are going to do that). Or maybe that's already your plan?

If needed, you could also do a separate dependency update first, then do the PHP-8.3-upgrade-with-another-dependency-update, to help keep each set of changes smaller. Just an idea, though I don't know if that would help.

Yes it's still in progress, but should be good to look at again. I'll need a final approval from you (@forevermatt) to remove the changes requested so I can merge this (date tbd). We'll have to do a quick sync with JAARS to ensure it still works and then roll back if it doesn't.

[mtompset]

Are the reconnections in the same test necessary? This was just cleaning up generally though, so it's okay.

The PHP dependencies were updated, so my requested changes were done.

[jason-jackson]

Are the reconnections in the same test necessary? This was just cleaning up generally though, so it's okay.

I believe they are setup for the actual test, and then the test. Probably better ways to handle it, but that was how they were written.