Skip to content

os2forms_f2 - #1

Merged
rimi-itk merged 19 commits into
mainfrom
os2forms_f2
Aug 25, 2026
Merged

os2forms_f2#1
rimi-itk merged 19 commits into
mainfrom
os2forms_f2

Conversation

@rimi-itk

@rimi-itk rimi-itk commented May 18, 2026

Copy link
Copy Markdown
Contributor

@rimi-itk
rimi-itk force-pushed the os2forms_f2 branch 9 times, most recently from 320428e to 759c978 Compare May 18, 2026 12:15
@rimi-itk
rimi-itk requested a review from jekuaitk August 24, 2026 10:17
@rimi-itk
rimi-itk marked this pull request as ready for review August 24, 2026 10:17

@jekuaitk jekuaitk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A couple comments and/or requests otherwise good!

Comment thread src/Plugin/WebformHandler/WebformHandlerF2.php Outdated
Comment thread src/Plugin/WebformHandler/WebformHandlerF2.php Outdated
Comment on lines +193 to +194
// Run only when submission is completed.
// @todo Run on update?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should add states as seen in OS2Forms/os2forms_get_organized#30 or OS2Forms/os2forms#333!

Comment thread src/Plugin/WebformHandler/WebformHandlerF2.php Outdated
Comment thread src/Form/SettingsForm.php Outdated
Comment thread src/Plugin/AdvancedQueue/JobType/F2.php Outdated
Comment thread src/Helper/WebformHelperF2.php Outdated
Comment thread src/Settings/AbstractSettings.php Outdated
final class WebformHandlerF2 extends WebformHandlerBase {
use StringTranslationTrait;

public const string ID = 'os2forms_f2_f2';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why the double "f2"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's the “f2” handler in the "os2forms_f2" module … I get your point … And this ID does not match the one actually assigned to the handler (cf.

).

Can you live with the change in d80a758?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Previously we haven't been above giving handlers the same id as the module (name) they live in ("os2forms_f2") although it may seem a tad strange, c.f. https://selvbetjening.aarhuskommune.dk/da/admin/os2forms_selvbetjening_overview/handlers . I can live with both ways as long as the ID's match!

Comment thread config/install/advancedqueue.advancedqueue_queue.os2forms_f2.yml Outdated
rimi-itk and others added 5 commits August 24, 2026 14:17
Co-authored-by: Jeppe Kuhlmann Andersen <78410897+jekuaitk@users.noreply.github.com>
Co-authored-by: Jeppe Kuhlmann Andersen <78410897+jekuaitk@users.noreply.github.com>
Co-authored-by: Jeppe Kuhlmann Andersen <78410897+jekuaitk@users.noreply.github.com>
Co-authored-by: Jeppe Kuhlmann Andersen <78410897+jekuaitk@users.noreply.github.com>
@rimi-itk
rimi-itk merged commit 6f89a58 into main Aug 25, 2026
4 of 9 checks passed
@rimi-itk
rimi-itk deleted the os2forms_f2 branch August 25, 2026 12:29
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