Skip to content

Updated digital post handler #3

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

Merged
merged 14 commits into from
Jan 28, 2025

Conversation

jekuaitk
Copy link

@jekuaitk jekuaitk force-pushed the feature/os2web_key_digital_post branch from 2182f4f to 1964c27 Compare January 22, 2025 13:59
@jekuaitk jekuaitk changed the title Updated Digital post command Updated digital post handler Jan 23, 2025
@jekuaitk jekuaitk force-pushed the feature/os2web_key_digital_post branch 2 times, most recently from 083b926 to a30a0cb Compare January 23, 2025 13:50
@jekuaitk jekuaitk force-pushed the feature/os2web_key_digital_post branch from a30a0cb to b0b898d Compare January 23, 2025 13:52
@jekuaitk jekuaitk requested a review from rimi-itk January 23, 2025 13:53
Comment on lines 19 to 20
$elementInfoManager: "@plugin.manager.element_info"
$webformTokenManager: "@webform.token_manager"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these really needed? Can we use

function __construct(
  …
  #[\Symfony\Component\DependencyInjection\Attribute\Autowire]
  $elementInfoManager,
  …
)

instead? Using an attribute may make the code easier to maintain.

Copy link
Author

@jekuaitk jekuaitk Jan 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. phpcs will deem this a possible useless override so i have ignored this specific sniff in the constructor.

@jekuaitk jekuaitk requested a review from rimi-itk January 27, 2025 11:56
/**
* {@inheritDoc}
*/
// phpcs:disable Generic.CodeAnalysis.UselessOverridingMethod

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you add the attributes in AbstractMessageHelper?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doh! Updated..

@jekuaitk jekuaitk force-pushed the feature/os2web_key_digital_post branch from 1c0337b to c0c053e Compare January 27, 2025 13:54
@jekuaitk jekuaitk requested a review from rimi-itk January 27, 2025 14:39
@jekuaitk jekuaitk merged commit 3ffd55b into feature/os2web_key Jan 28, 2025
5 of 6 checks passed
@jekuaitk jekuaitk deleted the feature/os2web_key_digital_post branch June 10, 2025 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants