Skip to content

Update deprecation notices #272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

bradenkeith
Copy link
Contributor

@bradenkeith bradenkeith commented Mar 28, 2025

Description

This PR will move deprecation notices from error_log to trigger_error. It will cause all deprecation notices in the unit tests to be expected and tested against. The test suite output should now be cleaner and considered 'passing'.

There's some issues with utilizing error_log in a package.

  • No Immediate Notification: error_log simply writes a message to the error log without integrating with the error handling system. This means that the user may never notice that a deprecated feature is being used.
  • Lack of Flexibility: With error_log, there’s no opportunity for developers to intercept or act upon the deprecation notice during execution.

Whereas, trigger_error will allow the developer to catch this notice. trigger_error() emits an error that goes through PHP’s error handling system. This means that deprecation warnings can be captured by custom error handlers or displayed in development environments.

Given that deprecation warnings are meant to notify developers without altering the core behavior of the package, this change should seen as non-breaking from an API perspective and can be added to a minor release.

Overview

Support

PHP >= 7.3 supports trigger_error.

Implementation

Instead of calling error_log(), developers of this package should call trigger_error($msg, E_USER_DEPRECATED).

This is the only change required.

if (!is_null($actors)) {
    $msg = "'actors' is deprecated. Please use 'actorNames' instead";

    trigger_error($msg, E_USER_DEPRECATED);

    $params["actors"] = $actors;
};

Developers of this package should continue to use the @deprecated docblock to help IDEs recognize the deprecation.

/*
* ...
* @var null|array $actors Actor names that Audit Log Events will be filtered by. @deprecated 3.3.0 Use $actorNames instead. This method will be removed in a future major version.
*/

Testing

There is a new TestHelper method called assertDeprecationTriggered that will accept two variables: The deprecation method you're expecting and the method call that is going to throw it. The test will fail if the $expected_warning is not thrown.

Example:

$auditLogExport = $this->assertDeprecationTriggered(
    "'actors' is deprecated. Please use 'actorNames' instead",
    fn() => $this->al->createExport($organizationId, $rangeStart, $rangeEnd, $actions, $actors, $targets, $actorNames, $actorIds)
);

The rest of the test will be evaluated as expected.

Future Considerations

PHP 8.4 introduced a Deprecated attribute. That would update our syntax.

#[\Deprecated(message: "use safe_replacement() instead", since: "1.5")]

The package currently supports >=7.3.0, so I did not implement this. If we want to future proof our deprecation notices, we could look at conditionally including the attribute. This felt too messy without much upside, so I am not attempting it.

Braden Keith and others added 18 commits March 11, 2025 13:05
…able, and add accessToken, refreshToken, and impersonator properties
And allow to be passed when creating or updating a user or organization.
And fix typo in getOrganization docstring
* Add WebhookResponse class for handling webhook actions and responses

* Refactor WebhookResponse create method and improve validation

* Resolve linting error

---------

Co-authored-by: Braden Keith <[email protected]>
…error for better error reporting. Update phpunit.xml to display details on deprecations and warnings. Enhance test cases to assert deprecation warnings for deprecated methods.
@bradenkeith bradenkeith requested a review from a team as a code owner March 28, 2025 19:55
Braden Keith added 3 commits March 28, 2025 16:42
…eOrganization' in Organizations class, and 'primaryEmail' in DirectoryUser class. Update tests to assert deprecation messages for these changes.
…rganizations class to provide clearer guidance for users. Add a test to assert the deprecation warning is triggered correctly.
…ent formatting and clarity. Adjust tests to reflect the updated messages for 'actors', 'primaryEmail', 'domains', and other parameters, enhancing overall error reporting.
bradenkeith and others added 6 commits April 2, 2025 20:53
* Improve Webhook and BaseWorkOSResource PHPDoc types

* Enhance Webhook class with improved code style and PHPDoc annotations

* Add accessToken and refreshToken to AuthenticationResponse class

* Update AuthenticationResponse class to include organizationId as nullable, and add accessToken, refreshToken, and impersonator properties

* Enhance OrganizationMembership class with additional PHPDoc properties for improved type documentation

* Add metadata and external id (workos#268)

And allow to be passed when creating or updating a user or organization.

* Add email standard attribute to DirectoryUser and mark deprecated standard attributes (workos#261)

* Add function to get organization by external id (workos#270)

And fix typo in getOrganization docstring

* Add support for creating, getting and updating users with external_id property (workos#267)

Co-authored-by: Eric Roberts <[email protected]>

* Bump to version 4.22.0. (workos#269)

* Structured responses to webhook events (workos#265)

* Add WebhookResponse class for handling webhook actions and responses

* Refactor WebhookResponse create method and improve validation

* Resolve linting error

---------

Co-authored-by: Braden Keith <[email protected]>

* Update deprecation notices in DirectoryUser class to include version information and improve clarity

* Update deprecation notices in Organizations class to include version information and improve formatting

* Update doc blocks for deprecation notices

* Update tests to expect Role Slug

---------

Co-authored-by: Braden Keith <[email protected]>
Co-authored-by: Eric Roberts <[email protected]>
Co-authored-by: Matt Dzwonczyk <[email protected]>
Co-authored-by: Pepe <[email protected]>
And allow to be passed when creating or updating a user or organization.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants