Updated folder methods#249
Conversation
Added Updated Folder Method
|
I know it's been a while since your PR. I'd like to clean up list, improve things and get a new release. Code has changed somewhat since your PR: https://github.com/Garethp/php-ews/commits/master/ Do you think it's still valid? Thanks! CC @kambereBr if you can opine) |
|
@jboomer007 are you trying to replace the magic method by removing the If so, I’m concerned this would cause a major regression:
That would break code relying on updates to other folder properties. FYI: Magic method removal has already been started in PR #265, but the goal should be maintaining the same capabilities, not reducing functionality. Thank you! |
|
I forked it and added a new method for updating folder name directly.
[brickwall-Logo]
James Boomer
816.674.5355
435 Nichols Road Suite 200
Kansas City MO 64112
From: Bruno Kambere ***@***.***>
Sent: Wednesday, August 27, 2025 8:21 AM
To: Garethp/php-ews ***@***.***>
Cc: James Boomer ***@***.***>; Mention ***@***.***>
Subject: Re: [Garethp/php-ews] Updated folder methods (PR #249)
[https://avatars.githubusercontent.com/u/29512462?s=20&v=4]kambereBr left a comment (Garethp/php-ews#249)<#249 (comment)>
@jboomer007<https://github.com/jboomer007> are you trying to replace the magic method by removing the @method UpdateFolder($request) annotation (line 78, ExchangeWebServices.php) and adding a concrete updateFolder() method instead?
If so, I’m concerned this would cause a major regression:
* Current magic method : full UpdateFolder SOAP support (any folder properties)
* Proposed change : hardcoded to only update DisplayName
That would break code relying on updates to other folder properties.
FYI: Magic method removal has already been started in PR #265<#265>, but the goal should be maintaining the same capabilities, not reducing functionality.
Thank you!
—
Reply to this email directly, view it on GitHub<#249 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AB2AOYWLSZB456DKXQGNS4T3PWWCFAVCNFSM6AAAAACE4KZ442VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTEMRYGE4DIOJXGM>.
You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>
|
|
While I think there might be scope to have a simpler method for updating folders, I think a method that updates a single property on a folder might be too narrow-scope to fit in with the simpler API usages that I've built. Taking a look at the other |
|
@jboomer007 @kambereBr any wisdom? :-) |
|
@jboomer007 wanted to check back in on this since it's been a while. So based on what @Garethp said, I think we can still keep the rename working, just not hardcode it to only the name. Basically do it the same way updateItems() works, you pass the change in yourself instead of it being fixed to one field. If you have time to update the PR that way, I'll be happy to see it move forward. If not just let me know and I'll open a new PR with this approach so it's not stuck forever. Either way works for me, just want to get this moving. Thank you! |
|
Here is the update that allows for you to pass an array of properties. Its a rough but works |
This has been updated. I gave a sample in the documentation for the function |
| return Utilities\ensureIsArray($this->getFolder($folderId->toArray(true))); | ||
| } | ||
|
|
||
| if ($response instanceof FolderInfoResponseMessageType) { |
There was a problem hiding this comment.
@jboomer007 thanks for the new commit. A few small things still need to be sorted out before we ask for another review from @Garethp
First, there's a bug here: you check $response instanceof FolderInfoResponseMessageType but I don't see that class imported anywhere at the top of API.php. That will throw a "class not found" or similar error the first time an update actually succeeds, you will need to add use garethp\ews\API\Message\FolderInfoResponseMessageType;.
| public function updateFolder(BaseFolderIdType $folderId, $foldersFields, $options = []) | ||
| { | ||
| $folderChange = $folderId->toArray(true); | ||
| $folderChange['Updates'] =['SetFolderField' => $foldersFields]; |
There was a problem hiding this comment.
Second, it's still only handling SetFolderField. To really match how updateItems() works, it would need to support DeleteFolderField and AppendToFolderField too, and ideally more than one field change per call. Right now it's wider than before but still a step short of that.
| // ("Value cannot be null. (Parameter 'participantResolver')") on | ||
| // cold/app-only mailboxes. The write succeeds, so verify the folder | ||
| // instead of failing outright; rethrow anything else. | ||
| if (stripos($e->getMessage(), 'internal server error') === false) { |
There was a problem hiding this comment.
Last one, the try/catch around "internal server error" worries me a bit. Matching on a string inside the exception message and then treating it as a success feels risky, that same message could show up for a real failure too and we'd silently swallow it. Can you say more about when you're hitting that error? Might be better to fix the actual cause than catch around it.
4 participants
No description provided.