Hi rahulkhandelwal1990,
This issue is about anonymous users.
We still have an open issue for authenticated users
🐛
Status field caching still broken
Active
Anw the two of us have an open discussionin
✨
Rework displaying dynamic information to JS for persistent caching
Active
Let's discuss over there.
Thanks a lot.
I did see the broken test after 'improving' the code base, but would not know how to fix this.
Please upgrade to new version 2.0.0.
Oh, I am working on 8.3 I think I must go back to 8.2, because I get more such problems reported.
Thanks, added.
The above commit should solve the problem.
Please test and report back. Then I will release a new version.
Indeed, I see many more problems:
- no problem with the 'Add exception' buttons
- both fields have the same title
- when first adding an exception one second field, then save, then add exception on other field, the value of first field is prefilled.
Regarding your remark #3.
I did change the $transition->to_sid and $transition->from_sid baseFields from 'entity_reference' to 'list_string'.
'list_string' does not need ->setSetting('target_type', 'workflow_state') .
ITMT, also because of your remark, I returned to the original, better 'entity_reference'
This should do it. Doing a string conversion of the 'options' already in module code, instead of relying on core.
Strange why a select option is treated different then a button title.
Off-topic: Sowieso, it is a crime to do the conversion between selectlist, radios, action buttons, and dropbuttons, espacially when using them in 'operations'.
Oh, I have it in my system, too.
Can it be a problem in the theme? My core themes give no problem. (since there is a lot of Twig calling involved)
Hmm, is this in module code on a node edit form with 'Workflow Transition form' field widget?
ITMT, upon your next module update, please run update.php and test the history view.
ITMT, upon your next module update, please run update.php and test the history view.
I realize this issue discussed more then one topic.
So, Indeed, it iseems best to close (and re-rename) this issue and regard only the original problem . Then the submit problem deserves a new issue.
If you don't mind, i will treat it as an edge case and a core issue for now. And will focus on getting a release 2.0.0 out.
Renaming the issue.
Thanks, it should now be corrected.
In FormBuilder::doBuildForm(), the following happens:
// If a form contains a single textfield, and the ENTER key is pressed
// within it, Internet Explorer submits the form with no POST data
// identifying any submit button. Other browsers submit POST data as
// though the user clicked the first button. Therefore, to be as
// consistent as we can be across browsers, if no 'triggering_element' has
// been identified yet, default it to the first button.
$buttons = $form_state->getButtons();
if (!$form_state->isProgrammed() && !$form_state->getTriggeringElement() && !empty($buttons)) {
$form_state->setTriggeringElement($buttons[0]);
}
$triggering_element = $form_state->getTriggeringElement();
Working on this.
I have an unlimited txt field and an unlimited file field.
Adding multiple texts, then save, indeed does not seem to save the initial transition.
In FormSubmitter::ExecuteSubmitHandlers(), the handels is "file_managed_file_submit", which is not used at all in my case.
If you use a Views display, the from_state and to_state fields must be deleted and added again, since the key is changed from 'target_id' to 'value'.
(but let me test)
The link is a 'list_string', in order to get proper widgets for this.
I might revert to entity_reference, but that will get 'autocomplete' widgets, too, in the workflow settings.
That seems like an oversight. Let me check.
Please try new dev version. the change might help.
I experienced the same thing on testing.
I found out that the weights on the workflow setting, stages list were identical.
Only if i dragged a state, the system renumbered the staties.
I assume the weights in your yml file are copied to the workflow states setting page.
But please check.
Regarding "- In v1.8, you could use workflow on an entity before saving it. In v1.9, the entity must be saved first."
I did notice this in the last week, and removed the code (thinking it was superfluous, and not the task of a transition to save the node), but did not realize it was a feature, too.
It could be reverted.
I would love to receive some help with that.
As a part time developer, the D8, 9, 10,11 changes require a lot of effort to keep up avoiding technical debt...
Did you run update.php.
Version 1.8 and 1.9 are about better usage of baseFieldDefinitions.
This also includes from state and to state.
If update.php does not work, you should remove and add the fields to the view.
IN #7:
- The function workflow_get_workflow_state_names() is missing in v1.9.
--> I can add this back
- The required parameters for WorkflowTransition::create() have changed a lot. e.g. without a wid value, no transition is created.
--> That should not be the case. I added the 'entity' as an input paramater, since that holds the $sid and $wid. Also $state holds $wid.
- In v1.8, you could use workflow on an entity before saving it. In v1.9, the entity must be saved first. If your code relies on the old behavior, you’ll need to adjust your logic.
--> Ofcourse, my test situation may be less complicated that yours.
- In v1.9, you must set an initial workflow status when creating a node. This wasn’t required in v1.8.
--> Before 1.8 upon node create, the widget has an initial value, being the 'first' state. it was removed shortly in dev, but not in 1.8. In current version, the 'first' state is set automatically. your contrib code should not be worrying about that.
- Some interfaces are missing.
--> Indeed, I introduced WorkflowTargetEntity and WorkflowRole, at some point, Roel and Entity should be subclassed/decorated.
- For some reason, I now sometimes get the workflow status as a system name instead of a label/value. When I switch back to v1.8, this problem goes away.
--> Is this only on the creation formatter? or also in other situations. I should have fixed this 2 weeks ago.
In #4, you should be able to use the standard 'Value' formatter, instead.
Indeed, I do see that the 'creation' state returns a machine_name, not a description. Not sure why.
I have added your proposed changes from #8.
Indeed, there are multiple changes in the code and interfaces.
Version 1.8 was an attempt to change the code to use more core code, i.c. using baseField definitions.
There are reports that this is crippled, also when using hooks.
And some changes (e.g., WorkflowManager functions moved to WorkflowTargetEntity) are indeed just moving code, for better DX in the long term.
I will need to take a look at your 3rd paragraph in #8 separately.
When creating a WorkflowTransition, it should still not be needed to have a to-state. In the past, the to-state could not be defined, in the new version, the to-state is defined (either first state when entity->isNew(), or else same state). Then, It can be overwritten by the widget.
Your change in the $this->entity_id might make a difference, since new Nodes have no ID, yet.
Indeed,
several interfaces have changed in v1.8 (which should not be used) and upcoming v1.9.
This is due to the implemenation of the baseFieldDefinitions of Workflow(Config)Transition.
I did not anticipate uon usage of function like above by third party modules.
(It seems you code can be replaced by workflow_state_allowed_values() )
Hmm,
you are using an outdated version.
I am about to release v1.9.
v1.8 is crippled (as you can see from some issues).
Please update to latest dev and report back. I will wait for your response, and then create a new -working- version
Also, please take a look at ✨ Rework displaying dynamic information to JS for persistent caching Active . Perhaps we can take this a step further.
Indeed,
that hook does not do your trick.
IIRC I never got reaction from the requester after implementing it.
I guess we hve 2 problems at hand:
1- the time is not properly formatted, according to timezone
2- the field cache is not correct for users from different time zones.
Let me then rephrase the title of this issue from:
"Rework displaying dynamic information to JS for persistent caching
to:
"Field cache and formatter incorrect when working with multiple timezones
ad 1- time format
The following hooks are available:
- hook_office_hours_time_format_alter()
- hook_office_hours_current_time_alter()
I guess you need a:
- hook_office_hours_timeslot_alter()
At the short term I cannot help you out with this seemingly complicated issue.
I invite you to check out OfficeHoursItem::formatTimeSlot() and propose a fix in that function or a calling function.
Please also check the code by searching on 'timezone' and also the
Issues →
It was determined that the timezone cannot be determined from within the module, but needs to be fed from custom code, since it is undefined where the appropriate timezone is set (user, entity, ...)
ad 2- field cache
As mentioned earlier, 'dropping the full page cache' is not the idea of the current implementation.
A lot of effort is put into a correct maxAge, after which , indeed, a full page load is triggered.
But for anonymous users, only the field itself is refreshed.
Please check OfficeHoursFormatterBase, lines 670-704.
There you can remove the checks for $this->currentUser->isAnonymous() and moduleExists('page_cache').
This influences the field caching and starts a JS statusUpdate for only the displayed field.
It normally only is activated for anonymous users, or when seasons and exceptions are in place.
(Bear in mind, that there is a check to 10 seconds in the JS, so debugging might influence the DX)
Also see 🐛 Status field caching still broken Active for an open issue, that I did not grasp entirely, but may be similar to your use case.
Please report back with your results.
It may be that we end up with a field setting 'always update the status, upon each field display'.
Thanks for all your help.
A small fix was added. A new release will be created soon.
No need for access, thank you.
The above should fix the problem.
The 'groupDays' messes around with the items. Not sure yet how that influences the other formatters, but moving the cloning of the items does the trick. Please check new version. I will release a stable release soon.
@artis, That is a nice page, BTW.
In my dev environment, I can only reproduce the error if the View does not only contains the normal 'Office Hours' formatter, but also the 'timeslot' field. There is no problem when adding the 'status' field, en did not test the 'season' field, yet. Probably, adding the normal formatter twice would generate the problem, too.
I did find how it happens, but am not sure yet why, or how to avoid it - working on that.
Your view does not have the 'timeslot' added. Do you have multiple office_hours fields in a resutl row?
OK, thanks, now i understand.
It was not clear to me that the problem happens in a Views display.
The hours are displayed correctly n a node list, but not in a Views Display. Strange...
Can you specify a test script?
Also, use latest dev.
Thanks fot your detailed reply.
I still cannot reproduce.have you Upgrade to v1. 26?
Nice! Does the error log report not give a message?
Please specify all settings for 'Manage display' of the field (only the ones that are directly visible - no matter the ones hidden in a field set).
Where do you specify the 'first day of week'? in the field, or in 'Regional settings'?
Are your hours through midnight?
And if you press 'Add time slot' on the Monday, the second slot is not hidden there?
Please find screenshots attached for admin/content view: pre and post Olivero, and for comparison also a Claro screenshot.
I have assured that this view has a full pager.
Please also take a look at the duplicate issues from #9
@decoppel, I can also not reproduce your problem, using the following script
- set 'action buttons' to Workflow config, make sure some transitions have a custom label
- create a node, add 2 files and 2 text items, press action button to save
- edit the node, repeat above.
I always see label(s), and node gets committed each time first time
Cannot reproduce on 'Workflow history tab', since the node fields are not on that page.
Cannot reproduce on 'node view' page for the same reason.
I will wait for spiderman's feedback and then create a new version.
I have some off-topic questions for this co-operative crowd:
- This module lacks tests. Do you have any automated tests to share in
#893294: Add tests →
?
- The attraction for this module is dropping, apparently since core has its 'Workflows' module. You are (still) using this module. You might want to share insights in
💬
Is Workflow redundant now that Content Moderation+Workflows is in core?
Active
I added the info to 💬 Is Workflow redundant now that Content Moderation+Workflows is in core? Active
Including info from 🌱 Comparing Workflow, Workflows, Workbench, Maestro, State Machine, Content Moderations, Workflows fields modules Active and marking that as 'duplicate'.
Please try again.
Sorry for the big commits, hiding the real changes.
Thanks for your efforts. I will keep this issue open for the upcoming contributions.
I did not take over all changes.
I did not update the .js file, since the changes do corrupt the functionality. Please test before contributing.
Please test the dev version and report back. I wil create a new version afterwards.
Strange, I cannot reproduce the error.
Is it dependent on a PHP version?
The patch loose the 'static' performatnce improvement.
Will the following work for you , too?
public function createItem($offset = 0, $value = NULL) {
$day = $value['day'] ?? NULL;
static $plugin_type = 'field_type';
static $pluginManager = NULL;
// Avoid PHP Fatal error: Constant expression contains invalid operations
$pluginManager ??= \Drupal::service("plugin.manager.field.$plugin_type");
@dcoppel, please test again.
Thoroughly, since the patch is bigger than expected. It also uncovered an incorrect workfllowSate::create().
Thanks for all your feedback and patience!
Thanks, fixed in #3514956-8: Introduce Hook classes (as per D11.1) → . Will create a new release in a minute.
Thanks. Version 1.26 is now released.
A new version 1.25 is released.
Thanks for your fast and efficient testing process!
A new version 1.25 is released.
Thanks for your fast and efficient testing process!
This is because of the field settings. You have set (some of the) the following checkmarks:
- Validate hours (Assure that endhours are later then starthours. Please note that this will work as long as both hours are set and the opening hours are not through midnight.)
- Require Start time
- Require End time
Please reopen if you think otherwise.
Please check new release 1.25 (to be released within hours)
Thanks, committed. Will create a new release today.
Thanks, this was an unfortunate typo.
This si sufficiently done in the last years.
johnv → created an issue.
Thanks, committed.
todo : - Title of 'workflow_state_formatter' does not adhere to field settings.
The 'new' view has column headers 'timestamp' and 'User ID'
This is normal. In Preview, the [save] button does not exist. User must return to original, and then save the entity.
Summary is updated accordingly.
Please specify all your Field 'Manage display' settings.
I tested with latest dev version (I was about to release this as new release, but will wait for your response) :
- Number of days to show = Show all days
- First day of week = - system's Regional settings - = Sunday
- Group consecutive days with same hours into one set = YES
This gives the following results:
Sun: Closed
Mon - Fri: 8:30-4:30
Sat: Closed
When changing First day of week to Tuesday, i get the following results.
Mon - Fri: 8:30-4:30
Sat - Sun: Closed
So, I cannot reproduce. Which version are/were you using?
No, that should not be the case. But I see our posts crossed. I Will take a look.
What do you mean with base field? Do you add the field programmatically to a programmed entity? Or as a normal field via field UI?
thanks for your better testing: do not test 0, 1 or 2, but also 3 times|
The cache was not properly updated, resulting an infinite recursive call.
Please try again.
Thanks a lot!
I did needeto add the extra directory for including the .inc files.
Please try again.
Let us rephrase: define user using EntityOwnerTrait.
Thanks for your answer.
Regarding the error, it seems it is another use case of the issue that was created just yesterday: 🐛 Drupal\Core\Field\WidgetBase::errorElement() Active
Please add your comments over there.