-
Notifications
You must be signed in to change notification settings - Fork 3
Digital Signature #167
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
base: develop
Are you sure you want to change the base?
Digital Signature #167
Conversation
* commit '841076066c102e5236808e7a9c020ca6092bc5cd': (37 commits) Release 3.20.0 Applied coding standards Added audit logging to FBS handler Added audit logging to Fasit Added webform id to digital post audit log messages Release 3.19.0 Adjusted audit logging message Added audit logging to os2forms_nemid Release 3.18.0 Added error handling upon saving XML Updated CHANGELOG Audit logged digital post Ensured audit log is installed Release 3.17.0 Cleanup Updated os2web_audit and os2web_datalookup versions Added further documentation Release 3.16.2 Applied coding standards Applied coding standards ...
* commit '7ace38332357344358eca1935f43ae2a30c23fa4': (31 commits) Release 3.22.1 Preparing 3.22.0 release OS-143 - Removed image_widget_crop Simplified validation Changed validation to check for empty string instead of null Conditional inclusion of patron phone number, since field is not required Updated changelog renamed fbs class Coding standards Add barn_tlf as an incoming field in FbsWebformHandler Handle phoneNumber assignment in advanced queue job Make phoneNumber a writeable property in Patron model Add support for phone numbers in FBS payloads Updating os2web_simplesaml to D10 compatible version Prepare release 3.21.2 Add missing iterable type specifications Run code analysis on fbs handler Add and use http-message-util Release 3.21.1 ITKDEV: Used configured format in maestro notification ...
* commit 'eecb47a101ea651f749c7f1bce1ea7313ae3cdce': OS-175 package version on status page Releasing 4.0.0 Updated CHANGELOG Update os2web_audit OS-115 Skipping address with empty matrikelnummer phpcs fix OS-115 Skipping address with empty matrikelnummer
@stankut, what is this pull request about and what is it trying to solve? Please provide a description and maybe a reference to a task with additional details. |
@stankut, please address the failing code checks (https://github.com/OS2Forms/os2forms/pull/167/checks). Most of them may be resolved by merging #168 (after review). |
Hi @rimi-itk. This task adds the ability to check/uncheck "Digital Signature" on the "OS2Forms Attachment" element and after submitting the form, sign the PDF file via "The Danish Digital Agency's signing component" (in danish: “Digitaliseringsstyrelsens signeringskomponent”). You can see more about the task in our documentation here: https://github.com/OS2Forms/os2forms_docs/blob/master/docs/digital_signature/documentation-on-digital-signature-module.md |
Hi @rimi-itk. We will review that on Wednesday, May 28th. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally a lot of good stuff. However, there are some inconsistencies in how things are done, e.g. when dependency injection is used and when static functions in \Drupal
are used. I have a feeling that some of the code has been written by a non-human programmer …
modules/os2forms_digital_signature/src/Plugin/WebformHandler/DigitalSignatureWebformHandler.php
Outdated
Show resolved
Hide resolved
modules/os2forms_digital_signature/src/Plugin/WebformHandler/DigitalSignatureWebformHandler.php
Outdated
Show resolved
Hide resolved
modules/os2forms_digital_signature/src/Service/SigningService.php
Outdated
Show resolved
Hide resolved
modules/os2forms_digital_signature/src/Service/SigningService.php
Outdated
Show resolved
Hide resolved
modules/os2forms_digital_signature/src/Service/SigningService.php
Outdated
Show resolved
Hide resolved
@rimi-itk As now phpcs is working again, there pop-up some warning from the other modules, not related with this PR. Namely os2forms_attachment |
modules/os2forms_digital_signature/src/Plugin/WebformHandler/DigitalSignatureWebformHandler.php
Outdated
Show resolved
Hide resolved
modules/os2forms_digital_signature/src/Plugin/WebformHandler/DigitalSignatureWebformHandler.php
Outdated
Show resolved
Hide resolved
modules/os2forms_digital_signature/src/Service/SigningService.php
Outdated
Show resolved
Hide resolved
modules/os2forms_digital_signature/src/Service/SigningService.php
Outdated
Show resolved
Hide resolved
Your "branch is 18 commits ahead of, 9 commits behind develop." You probably need a merge/rebase with |
* develop: Added docker compose setup Update .github/workflows/pr.yml Cleaned up code Cleaned up code Cleaned up changelog Fixed digital post command. Updated GitHub Actions steps.
Last issues fixed, @rimi-itk please have a look now |
@stankut, I know that GitHub is good at hiding comments ("conversations") on code and it's easy to loose sight of what has been adressed and what hasn't, but please address all comments, e.g. #167 (comment), and use "Resolve conversation" when done. We should use dependency injection whenever possible – and in general be consistent on how we do things. |
* develop: Release 4.1.0 #4232: Normalized composer Updated CHANGELOG Added patch allowing attachments in coc_forms_auto_export Removed old patches Removed old patches Updated composer
@rimi-itk think i checked all the comments, please have a look |
No description provided.