Account created on 4 August 2020, over 4 years ago
#

Merge Requests

Recent comments

Sorry, to clarify, should views.filter.schema.yml in webform_workflows_element_views be renamed to webform_workflow_element_views.views.schema.yml?

Tweaked a bit to make it render any errors it might have to help people debug their setup.

What is the "webform_event_dispatcher" module? I can't find reference to it anywhere. It looks to be an issue with that module rather than this one however.

$access_check provided as optional parameter. Did not apply merge request for reasons stated, and this approach covers off the first use case.

2) Access checks should be done as part of route access checking anyway. This allows menu items that shouldn't be accessible to a user to be hidden and access prevented, rather than requiring additional code to say "Sorry. We showed you this menu item but you can't actually use it." It also makes the code better structured and more maintainable.

As mentioned we need to be able to access check at different points, not just at the routing level. Without this approach, it'll end up less maintainable as we'd need to repeat the access check logic across functions I think.

Thanks. This is more simply resolved by a fix already in the latest dev, which also allows using the admin log as a token. It seems to not require being a 'hidden' element, just to have some value set even if an empty array.

    $elements['workflow_fieldset']['log_public'] = [];
    $elements['workflow_fieldset']['log_admin'] = [];

This is already resolved in the latest dev, by checking if $workflowType is false. I have credited both however. There will be an alpha4 soon that will incorporate this.

Hi @mably - I'm happy to add you as a maintainer, however I have also been working on an update (sadly I have not had time to maintain as frequently as I would like due to work, so it's slow going).

I will complete that release over the next week or two, incorporate any mergable patches such as the ones you added, and then once that's clean add you as a maintainer so you can do releases as necessary. I'll update and close here once you've been added. I will do this by the 6th of October at the latest.

As an aside, I think it'd be good to create an issue with a roadmap at some point for a proper release - I imagine Drupal 11, automated tests as a minimum and will have a think about others.

I don't think this should be considered closed - could maintainers re-open? I think using the default store in this case is a perfect idea for the use case.

Updated patch for latest version.

While I understand the need for better issue reporting, in this case it's a small change that is evident from the title.

Hi, I've incorporated the changes from the patch and some other minor bug changes and done a D10 release. Let me know if any issues.

+1 to NicholasS - I'd rather use symfony_mailer, but this issue might mean we have to use https://www.drupal.org/project/symfony_mailer_lite and keep using mailsystem etc.

Hi, I've done an updated release with D10. We've been using this in production for a while but it doesn't have automated tests or anything so worth checking comprehensively if it's all working for you.

Ah, it only exists in the dev version. If you're using the current alpha, you may need to use runTransitionOnElementValue and then look at the logic in the dev version for runTransition to do the transition on the element: https://git.drupalcode.org/project/webform_workflows_element/-/blob/1.0....

I appreciate you've found an alternative, but for sites which need to use the one CKEditor accordion across both CKEditor and Gutenberg, your original request would be valuable.

If you'd like to not receive notifications for this, I think unfollowing it should do that despite you being the creator of the issue.

Hi @liam-morland, I'm happy to become a maintainer to manage the Drupal 9/10 version based on what I've developed. We've been using it in production for a while, so it is largely working, but needs a lot of work in terms of docs and tests. Also I'm happy to resolve @SocialNicheGuru issues in that branch.

Hi swirt, really cool module and I can see the use cases for it might be overlapping in some cases. I've added it to the "See also" with a clarificatory note.

The module is designed to use transitions as part of the core workflows module, so you'd need to transition the state from one to the other, rather than simply change the workflow state. So, you'd have a transition from "unpaid" to "paid", say "mark as paid", and you'd run that on the submission at the point of payment.

Then, there is a function on the workflows manager service to help do that:

    $workflowsManager = Drupal::service('webform_workflows_element.manager');
    $element_id = 'workflow';
    $transition_id = 'mark_as_paid';
    $log_message = 'Paid in order ' . $order->getOrderNumber();
    $success = $workflowsManager->runTransition($webform_submission, $element_id, $transition_id, $log_message);
    if ($success) {
      $webform_submission->save();
    }

Hi @solideogloria, not sure why but your approach broke any views pages that used anything other than the 'Table' format, and broke the "Webform submission operations bulk form" checkboxes (they no longer appeared). I am not sure why (and don't currently have time to test) but I have reverted to my original approach and it continues to work.

This has likely been introduced in this commit.

There are no code changes to course_quiz in that commit (just to a test), so I think this is something across multiple types of course object. However from a quick look I can't understand what other changes might have done this.

In the meantime if anyone needs the working dev, perform:

composer require drupal/course:dev-3.x#287a3bf

Modifying package.json drupal-js-build to

"drupal-js-build": "github:front/drupal-js-build#468f4565916b3a6d086710e646c74d462646a450",

Appears to fix it. See https://github.com/front/drupal-js-build/issues/21

Thanks! I am going to close this as fixed, to avoid confusion. If there are further 6.2 compatibility issues (haven't found any myself with a quick check) please create new issue(s).

Ah, I see I do have credit on the issue, but was not credited as part of the git commit message.

Hi - the merged request appears to be largely mine with some modifications. Could I please get shared credit on it?

I've created a merge request with a new formatter - OfficeHoursFormatterSeparateTables, "Tables, split by season and exception". Where there are seasons and/or exceptions, the main hours are given the caption 'normal hours', and the seasons or exceptions are given their relevant labels.

I would suggest that separate tables is actually what I would have expected in terms of both UX and DX for this functionality, so this new formatter could just replace OfficeHoursFormatterTable anyway, but I've created it as a separate formatter for now.

Great idea, thanks and sorry for the delay incorporating it. I've added an optional element ID field and a log option.

In future I think we can have it play nicer with VBO by loading available workflows and transitions based on the webform(s) of the submissions, but I'm not sure how to do that in a performant way right now.

I agree getCompositeElements is pretty overloaded, from memory this is due to the way custom composite elements are set up. In any case I think your patch works well for this case - thanks!

Hi. I've not used this patch as $options could be non-empty but still not have [''] as a key, so the issue would continue. I've committed a change to dev to resolve this, could this please be checked in your environments? (I can't recreate this issue myself sadly).

There appear to be competing patches here, five minutes apart. Admire the keenness!

I've incorporated both, noting:

- MikaT - re: !$workflow instanceof WebformWorkflowsElement, $workflow would never be the element there, it'd be the Workflow entity itself
- PrabuEla - getStatesFromElement was set to return an array (i.e. was not nullable) - fixed

I've also done the same treatment for any calls to getWorkflowTypeFromElement as that would lead to the same issue.

Not to resurrect, but I've created a contrib module to utilise the Inline Entity Form module for this purpose. It's a bit rough and ready currently but hopefully a start.

https://www.drupal.org/project/webform_inline_entity_form

Can you please provide more detail so we can reproduce it? Possibly include screenshots and the webform source YAML if you can. I have done what you have described in the initial issue, with English and French, and it has not had an issue.

There are two issues with conditions in the current dev/beta:

  • The one above, which is making another element conditional upon the selected transition
  • The available conditions on the workflow element to make a transition conditional upon another element

I've clarified the title of this one and will create an issue for the second one.

I've tested this on a clean site having added a new language, switching both the user language and the site default language, and can't reproduce the issue. This may have been resolved since the issue was posted. If not please let me know.

(1) Fully agree with needing this ability, was not envisioned as part of writing this. See below proposed approach.

Re: (2) - access checks are not just routing. They also need to be done on the element level, and the transition option level, for webforms generally and for this module to provide a good UI for selecting available transitions. I know in the other issue you raised (now resolved) it could be done in routing, but here the user can have full access to the route, but not necessarily access to every workflow element, and for each element they do have access to, not necessarily access to every transition. It needs to be (able to be) checked at some point separately from routing.

Currently this check in runTransitionOnElementValue prevents someone programmatically running a transition via this helper function that the user does not have access to, as you say. We can easily make that an optional access check, to provide an easy way to safely run the transition, or 'unsafely' run it programmatically if needed.

So I think the simplest compromise right now is to add an optional parameter to the function, $access_check, which is by analogy to how entity queries can disable the access check. Here, it will default to checking access, to avoid unintended consequences, but allow overriding.

If at some point we identify every possible case where runTransitionOnElementValue is used and ensure there is an access check of some sort in front of it, I think it's still good DX to have an optional access check in this function for doing things programatically, so we could just change the default for $access_check to false.

Anyway. I've added $access_check in the latest dev for now. Happy to discuss as I'm sure I've missed something, development of this module is in fits and starts due to my workload and my planned approaches may have drifted off a bit.

The second issue is resolved in the latest dev. I can't recreate the first issue, but I think it may have been resolved earlier anyway.

Production build 0.71.5 2024