intacct / intacct-sdk-php

Official repository of the Sage Intacct SDK for PHP
https://developer.intacct.com/tools/sdk-php/
Apache License 2.0
25 stars 33 forks source link

Patch php 8.2 #187

Closed thestalnakergroup closed 3 months ago

FutureShirtsDevs commented 1 year ago

I would love for this to get merged to stop deprecation warnings in 8.1. Only thing I see is startElement should "return" parent::startElement($name);

thestalnakergroup commented 1 year ago

image Getting this to merge. Looks like someone with write access have to approve?

thestalnakergroup commented 1 year ago

@daemionfox can you merge this in?

thestalnakergroup commented 1 year ago

@jimmymcpeter any chance we can get this merged?

jimmymcpeter commented 1 year ago

@jimmymcpeter any chance we can get this merged?

Unfortunately I don't work in this area anymore. Let me reach out to some folks and see if they can help.

thestalnakergroup commented 1 year ago

Thank you!

[photograph]

Billy Stalnaker Owner

M: (910) 232-4703 E: @.*** The Stalnaker Group stalnakergroup.comhttp://stalnakergroup.com/


From: James McGonegal @.> Sent: Friday, June 23, 2023 1:15:43 PM To: intacct/intacct-sdk-php @.> Cc: William Stalnaker @.>; Author @.> Subject: Re: [intacct/intacct-sdk-php] Patch php 8.2 (PR #187)

@jimmymcpeterhttps://github.com/jimmymcpeter any chance we can get this merged?

Unfortunately I don't work in this area anymore. Let me reach out to some folks and see if they can help.

— Reply to this email directly, view it on GitHubhttps://github.com/intacct/intacct-sdk-php/pull/187#issuecomment-1604578364, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AYIHHYF7CXPZ5TNCYBN3WKTXMXFL7ANCNFSM6AAAAAAVADSPQU. You are receiving this because you authored the thread.Message ID: @.***>

chris114782 commented 1 year ago

One final change, Intacct\Xml\Response\ErrorMessage::cleanse() should be refactored as FILTER_SANITIZE_STRING was deprecated in php 8.1 - The suggested alternative is htmlspecialchars() which looks like it would suffice in this case but is not a direct drop in replacement in terms of behaviour as it does not strip tags, just encodes them.

thestalnakergroup commented 1 year ago

@cstretton, merged master to get the return in there. Also, asked ChatGPT to refactor the function you referenced. I never got any issues with this function, and I'm not exactly sure how to test to make sure it works correctly, but it looks right to me.

Thanks for your help!

rijnhard commented 1 year ago

@jimmymcpeter you find anyone internally who can approve/test/merge?

rijnhard commented 1 year ago

@jimmymcpeter any chance we can get this merged?

Unfortunately I don't work in this area anymore. Let me reach out to some folks and see if they can help.

Just following up here if you could get this through to the right people?

jimmymcpeter commented 1 year ago

@rijnhard I think your best bet is to fork the repo, make your fixes there, and update your composer.json to load from your repo instead -- https://getcomposer.org/doc/05-repositories.md#loading-a-package-from-a-vcs-repository