🇳🇱Netherlands @johnv

Nice,
I updated the issue summary with the findings and changed it into a Documentation topic. Leaving it open for other to find it quickly.

Release workflow 2.1.7 has been created.
Is the problem still there?

Release workflow 2.1.7 has been created.

Release workflow 2.1.7 has been created.

Release workflow 2.1.7 has been created.

The current working branch is 2.1.x
Haven't been able to test , yet.

The simplest way in admin UI to restore the original Views display would be to delete the View and import it again.
- Go to /admin/structure/views/view/workflow_entity_history and choose 'Delete view'
- Goto /admin/config/development/configuration/single/import and select the configuration type View.
- Copy and paste the content of the file https://git.drupalcode.org/project/workflow/-/blob/2.1.x/config/optional...

Backup your own Views.yml first and then check the differences.
- Goto admin/config/development/configuration/single/export

Or compare the current view with the settings in attached screendump.
I guess the problem is with the new arguments.

The first patch also removed the titles of the Exceptions and Seasons. This is now corrected.

Just for the record. My attempts until now in the attached patch.

Also set 'Revert' to first place in view's operations dropdown buttons, to make it more visible.

After several attempts to set proper caching on Views row caching, just disable the caching. The views should be very fast anyway.

Version 3.0.2 is released.

Thanks.

I invited Finn Lewis (original poster) to join the discussion here.

Finn, please check the changes, and if the last pending patch makes any sense for you.
I think all this only makes sense if there is full coverage in UnitTesting?

Thanks a lot. This was a long time annoying bug.
The resulting patch also prevents 'null' records to be coming in.

Thank you for your efforts. Please see my attached patch (sorry for not using MR).

That patch only contains a one-line change (thanks to your efforts, though).
Please review, and

[stat preparations]
- set 'number of slorts per day' to 3;
- edit node, add 2 exceptions and save.
[start test]
- edit node again,
- add exception, 3 times
- fill 2nd exception with 1 or more timeslots,
- add exception, --> too many slots are added.

Does that happen with your patch, too?
Even if so, i guess that can be considered as an edge case we can leave alone.

Regarding your patch:
- please do not rename variables, it makes the patch harder to understand.
- the fallback to NULL, do you encounter an erroro if not, in if (($dv['day'] ?? NULL) !== 'exception_day_delta') { ?
- teh check for array, do you encounter an arrray if not, in if (is_array($default_values)) { ?

Did you update/change the Views display 'workflow_entity_history'? Please reset and see what happens.

Thank you, but is is not correctly working when the entity already as 1 or more exceptions saved.

This should do it.

johnv → created an issue.

johnv → created an issue.

johnv → created an issue.

Setting this to Active again, since there is a need to add above test for PHP8.2, D10.2

Please update to workflow version 2.1 instead.

Also, I cannot find any documentation or change-record on hook_modules_loaded().

OK, niow that you have added some new transitions, please check "workflow_transition_history" table.
I guess all records have valid records.

When saveing the transitions, there might be a mismatch between the 'target_id' and the 'value' of the Itemlist item. But that does not seem the case.
So, the problem is in the reading of the table, and the translation from DB-column to WorkflowTransition values.

The Workflow History page is served by WorkflowTransitionListController.php.
Here, you find 2 options, hardcode toggled by $use_views_instead_of_list_builder = TRUE; :
- the view is generated by a hardcode list - this is disabled by defalut, and it only there as a fallback and for testing.
- the view is generated by a Views view display 'Workflow Entity Hitory'.

Did you update/change the view? Please reset and see what happens, or, safer, remove and re-add a from/to State column, and upon success, the other one.
If no success, you may set the toggle for testing.

Please confirm if current codebase deserves a new version, of if you think it needs a bit extra

Thanks, I created
✨ Add lanuguageicons to Widget values Active
and
✨ Add SelectWidget formatting options to AutocompleteWidgets Active

johnv → created an issue.

johnv → created an issue.

Thanks. committed.
The patch is a bit bigger than original, due to making code from 3 widgets look more alike.

I avoided some code by inheriting from StringTextfieldWidget.
I also tried to add the formatting settings from the SelectWidget to the Automcomplete widget, but failed for now, since LanguageAutocompleteController does not have the correct information. It has no access to the widget settings or the list of languages.

Committed with credits in the issue. Thanks.

The patch became much bigger when testing:
- adding defaultSettings();
- encapsulating the value formatting in one function, both for formatter and widget.

Icons are still not supported in widget. I guess that is not possible, since select widget supports only a ascii string?

Thanks.

Coming back to #15. Is this possible?
"How to add a test for D10.2 with PHP8.2 ?"

THanks, I created an issue for later follow-up: 💬 Offering to be project owner/administrator Active

johnv → created an issue.

Committed.
The issue title does not coincide with the patch contents "git commit -m 'Issue #3542512 by bluegeek9: Add tests for office_hours_update_xxxx() functions'"

Thanks, committed. Let us await the jobs.

You are fast, I assume you have some tool?

I have changed version naming for workflow, workflow_notifications, and will do this for office_hours, languagefield.
For those I am de facto the only developer.

"Some of the Hooks have non camel casing. I can fix it but wasn't sure if the change will be welcomed."
Fixing that would be nice. Thank you.

I really appreciate your efforts. I can learn from that and build upon it!

THIsis committed here: https://git.drupalcode.org/project/workflow/-/commit/fd68bd4e6e10c61c54e...

wow, what a relief. The below difference may not seem so much, but it will make a big difference on my other modules, too.
I now will add other errors at some other time, if needed. Thanks.

before: https://git.drupalcode.org/project/office_hours/-/jobs/6276228

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
-----------------------------------------------------------------------
    SOURCE                                                        COUNT
-----------------------------------------------------------------------
[x] Drupal.WhiteSpace.ScopeIndent.IncorrectExact                  30
[x] Drupal.Commenting.PostStatementComment.Found                  14
[ ] Drupal.Semantics.FunctionT.NotLiteralString                   12
[ ] Drupal.Files.LineLength.TooLong                               11
[x] Drupal.Commenting.InlineComment.InvalidEndChar                9
[x] Drupal.Classes.UnusedUseStatement.UnusedUse                   8
[ ] Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps  8
[x] Drupal.Commenting.InlineComment.NotCapital                    7
[ ] Generic.CodeAnalysis.UselessOverridingMethod.Found            6
[x] Drupal.Commenting.InlineComment.SpacingAfter                  5
[ ] Drupal.Semantics.FunctionT.ConcatString                       4
[ ] Drupal.Commenting.DocComment.MissingShort                     3
[ ] Drupal.Commenting.FunctionComment.ParamMissingDefinition      2
[x] Drupal.Commenting.FunctionComment.SeePunctuation              2
[x] Drupal.Classes.ClassDeclaration.CloseBraceAfterBody           1
[ ] Drupal.Commenting.DocComment.ParamGroup                       1
[ ] Drupal.Commenting.FunctionComment.MissingReturnComment        1
[ ] Drupal.Commenting.FunctionComment.ParamNameNoMatch            1
[x] Drupal.Commenting.FunctionComment.ReturnVarName               1
[x] Drupal.Commenting.InlineComment.NoSpaceBefore                 1
[x] Drupal.Commenting.InlineComment.SpacingBefore                 1
[x] Drupal.Commenting.VariableComment.IncorrectVarType            1
[x] Squiz.WhiteSpace.FunctionSpacing.AfterLast                    1
[x] Squiz.WhiteSpace.OperatorSpacing.NoSpaceBefore                1
-----------------------------------------------------------------------
A TOTAL OF 131 SNIFF VIOLATIONS WERE FOUND IN 24 SOURCES

After: https://git.drupalcode.org/project/office_hours/-/jobs/6277048

PHP CODE SNIFFER VIOLATION SOURCE SUMMARY
-----------------------------------------------------------------------
    SOURCE                                                        COUNT
-----------------------------------------------------------------------
[x] Drupal.WhiteSpace.ScopeIndent.IncorrectExact                  30
[ ] Drupal.Semantics.FunctionT.NotLiteralString                   12
[x] Drupal.Commenting.InlineComment.InvalidEndChar                9
[x] Drupal.Classes.UnusedUseStatement.UnusedUse                   8
[ ] Drupal.NamingConventions.ValidFunctionName.ScopeNotCamelCaps  8
[x] Drupal.Commenting.InlineComment.NotCapital                    7
[ ] Generic.CodeAnalysis.UselessOverridingMethod.Found            6
[ ] Drupal.Semantics.FunctionT.ConcatString                       4
[ ] Drupal.Commenting.DocComment.MissingShort                     3
[ ] Drupal.Commenting.FunctionComment.ParamMissingDefinition      2
[x] Drupal.Commenting.FunctionComment.SeePunctuation              2
[x] Drupal.Classes.ClassDeclaration.CloseBraceAfterBody           1
[ ] Drupal.Commenting.DocComment.ParamGroup                       1
[ ] Drupal.Commenting.FunctionComment.MissingReturnComment        1
[ ] Drupal.Commenting.FunctionComment.ParamNameNoMatch            1
[x] Drupal.Commenting.FunctionComment.ReturnVarName               1
[x] Drupal.Commenting.InlineComment.NoSpaceBefore                 1
[x] Drupal.Commenting.InlineComment.SpacingBefore                 1
[x] Drupal.Commenting.VariableComment.IncorrectVarType            1
[x] Squiz.WhiteSpace.FunctionSpacing.AfterLast                    1
[x] Squiz.WhiteSpace.OperatorSpacing.NoSpaceBefore                1
-----------------------------------------------------------------------
A TOTAL OF 101 SNIFF VIOLATIONS WERE FOUND IN 21 SOURCES

johnv → created an issue.

Let us see what happens.

I am the only maintainer. How do I promote myself?

And:
how to add a test for D10.2 with PHP8.2 ?

Thanks.

Nice page, #10.

Can you also tell me how to change the default branch (not yet for this module, but already for e.g., workflow module) ?
I cannot find it. And Google and drupal.org give me outdated answers.

Yeah, I know, do not try to resolve them.
Ther are stupid rules:
- max 80 char per line
- no empty line after inline comments
- no inline comments after a code line.
- code in inline comments is regardes as text.

Unless you can tell me a way to exclude such nonsense from the phpcs jobs, i will leave it this way.

Thanks, anyway

THanks, this was a blocker for 📌 Modernize from Annotations to Attributes Active

Thanks,
just committing as-is, and awaiting what happens with it.

Above commit is kindly copied from 📌 RenderElement and FormElement base plugin classes are deprecated in 10.3 and removed in 12.0 Postponed by @bluegeek9

the following is added to the summary:

[...] According to this drupal core change-record:
RenderElement and FormElement base plugin classes are deprecated and renamed to RenderElementBase and FormElementBase →

Oh,
is there a backslash missing?

if (class_exists('\Drupal\Core\Render\Element\FormElementBase')) {
  class_alias(
    '\Drupal\Core\Render\Element\FormElementBase',
    'Drupal\office_hours\Element\FormElementBase'
  );
}

Thanks. I will use it in my other modules, too, like workflow module.

This was blocked by 📌 RenderElement and FormElement base plugin classes are deprecated in 10.3 and removed in 12.0 Postponed , which is now resolved.

johnv → created an issue.

I committed the README file in 📌 Update README file Active .
Thanks.
I made a small change by referring to workflow.api.php file instead of copying the information.

johnv → created an issue.

Please download newest dev version, make sure your hook_update number is correct, run update.php and test again.

The problem may also be that your Hook_update() number is 9001.
That means that any hoop_update is provided by the module with a LOWER number, will NOT be implemented.
Please make sure that you hook_update number reset. (I am not sure how to do that).

Sorry,
I misunderstood #16's "I confirmed the symptom was reproduced" for "[..] the symptom was fixed"..

I will also implement #8. Since for a short while the 'forced' field was not defined as computed.
Apparently the workflow_update_8007() does ot work for your situation.

I do not understand the parameters of this last test. They are now back to original state.

Indeed, thanks for your information.
I credited the Issue, which will reflect on your user page. It is too late for the commit.

It all started with supporting 'baseFieldDefinitions' in the WorkflowTransition. But that infected the FormElement.
Like doing a simple change in your old house...

I will just commit this and see what happens.

there were the following issues:
- EventDispatcherInterface definition;
- no trailing periods in inline comments;
- wrong parametes in $this->assertTrue($this->workflowTransition->isAllowed($this->account, $from_state, $to_state));

That will help with this 14 year old issue: #893294: Add tests →

Wow, that was a long way!
Thanks a lot for your cooperation an patience!

I guess we can close this issue.

Thisis a follow-up issue from 📌 Hook classes are introduced in D11.1 Active .
The hook_preprocess features are changed in D11.2

Yes, let's remove it altogether.

The code is now commented out, after encapsulating the code better.

It is only needed in ListWidget in edge cases.

Thanks,
the error was in the declaration of the called function. I fixed all interfaces of the function in the class.

I could not reproduce a case for the first half of your patch.
Please download dev version and test again.

Indeed, this is a known bug. Not sure how to solve this, either.

I made the code more explicite to handle all cases:
- a field name is given yes or no
- an entity has 0, 1, multiple workflow fields.

Please check.

Ow, 8 errors, that is not progress...

I reverted the code structure of WorkflowHistoryAccess::access(). The new code introduced a gap.

The misspelled 'workflow_transion' exists already for years, see #2927556: Correct typo on permission "access ... workflow_transion overview" (D9) → .
I think it exists since the start of the D8 version.
Are your tests failing on that?

It is annoying, but changing that would need a hook_update_xxx().
Both are not typo's since the real typo is in file WorkflowPermissions.pgp:

      // D7->D8-Conversion of 'workflow history' permission on Workflow settings to "access $type_id overview" (@see NodePermissions::access content overview).
      "access own $type_id workflow_transion overview" => [
        [...]
      ],
      "access any $type_id workflow_transion overview" => [
        [...]
      ],

And that is being referred to.
I do not really know how to fix that without breaking existing installs.

johnv → created an issue.

This should do it. Please test.

This is now included in dev version 2.1.x

johnv → created an issue.

And the above concludes the processing of your input.
Please give me a day for some additional changes, then test again with 2.1.x dev version.
Thanks.

Above commit handles the timestamp. I now understand the goal: simpler code.
I avoided the $formstate input vs. values discussion (which I always tend to struggle with), by re-using the result from valueCallback.

I will create some follow-up on the timestamp, which is outside the scope of your patch.

I updated workflow_current_user() for anonymous users.
I do not know how to test CLI context, but I presume there is always a user, right? I may be a user with ID = 0.
I did not apply below chunks, since getOwner() should return an object for user 0, too.
And workflow_current_user() is now better.

diff --git a/src/Entity/WorkflowTransition.php b/src/Entity/WorkflowTransition.php
index 02b0bdd..83d78f3 100644
--- a/src/Entity/WorkflowTransition.php
+++ b/src/Entity/WorkflowTransition.php
@@ -18,6 +18,7 @@ use Drupal\Core\Session\AccountInterface;
@@ -709,7 +712,8 @@ class WorkflowTransition extends ContentEntityBase implements WorkflowTransition
@@ -1053,7 +1051,7 @@ class WorkflowTransition extends ContentEntityBase implements WorkflowTransition
@@ -1447,4 +1445,21 @@ class WorkflowTransition extends ContentEntityBase implements WorkflowTransition

Now working on the $user/ $owner.
The $timestamp is left. There were no comments about that, only code. Was/is there an error in that?