When I create a translation of a node using entity translation, with the date field set as NOT to be translated, it deletes the date field records corresponding to the entity from the database (i.e. in field_data_field_date and field_revision_field_date). Additionally the following error message is fired:

Mensaje de error
Notice: Undefined index: field_year en date_combo_validate() (línea 330 de /home/me/web/example/modules/date/date_elements.inc).

The date can only be recuperated by filling it in once more on the original node edit form. Every subsequent edit of the translated node deletes the data again.

CommentFileSizeAuthor
#77 date-multistep-defaults-1178716-77.patch687 bytessreese
#52 date-use-form-values-instead-input-patch-1178716-52.patch10.26 KBdas-peter
#48 date-use-form-values-instead-input-patch-1178716-48.patch9.48 KBdas-peter
#46 date-use-form-values-instead-input-patch-1178716-46.patch7.3 KBdas-peter
#44 date-use-form-values-instead-input-patch-1178716-44.patch7.3 KBdas-peter
#39 date-use-form-values-instead-input-patch-1178716-39.patch5.65 KBdas-peter
#37 date-use-form-values-instead-input-patch-1178716-37.patch5.35 KBdas-peter
#34 date_entity_translation-1178716-34.patch5.16 KBdboulet
#30 date-use-form-values-instead-input-patch-1178716-29.patch22.74 KBdrasgardian
#27 date-use-form-values-instead-input-patch-1178716-27.patch22.74 KBdas-peter
#27 date-use-form-values-instead-input-patch-1178716-27-no-space-diff.patch5.1 KBdas-peter
#27 date-use-form-values-instead-input-patch-1178716-27-ultimate-whitespaces-fix.patch160.53 KBdas-peter
#23 date-use-form-values-instead-input-patch-1178716-23.patch21.5 KBdas-peter
#21 date-use-form-values-instead-input-patch-1178716-21.patch11.98 KBdas-peter
#21 date-use-form-values-instead-input-patch-1178716-21-without-whitespaces.patch2.69 KBdas-peter
#8 date-translating-deletes-date-field-entry-1178716-8.patch2.2 KBdas-peter
#8 date-translating-deletes-date-field-entry-1178716-8-whitespace-fixes.patch10.98 KBdas-peter
#6 date-translating-deletes-date-field-entry-1178716-6.patch821 bytesdas-peter
#6 date-translating-deletes-date-field-entry-1178716-6-whitespace-fixes.patch5.71 KBdas-peter

Comments

renat’s picture

Confirm, I faced the same problem. Quick search show us this tread: http://drupal.org/node/1021512

kdebaas’s picture

In an attempt to debug this myself, i installed a clean drupal 7 site, installed date and entity_translation, and added a second language.

Translating a node with a date field attached still deletes the date. The error message is not fired though, so it may be unrelated.

Unfortunately I still can't figure out the cause of this bug.

renat’s picture

Situation changed a bit after all this great work done by Date maintainers in the last days. Now we will receive this error messages, when we are trying to translate entity with untranslatable Date field:

Notice: Undefined index: value in date_field_validate() (line 304 of /var/www/test2.sky37.com/public_html/sites/all/modules/date/date.field.inc).
Warning: DateTimeZone::__construct() [datetimezone.--construct]: Unknown or bad timezone (E) in DateObject->__construct() (line 103 of /var/www/test2.sky37.com/public_html/sites/all/modules/date/date_api/date_api.module).
Notice: Undefined index: value2 in date_field_validate() (line 305 of /var/www/test2.sky37.com/public_html/sites/all/modules/date/date.field.inc).

Hope this message will help with debugging.

karens’s picture

Status: Active » Postponed

Just note that there are problems around field translation that are out of my control. See #1164852: Inconsistencies in field language handling and #1205504: Uninstalling Content Translation disassociates fields. Until the core problems are fixed, it's hard to tell if there is a bug in Date or not.

renat’s picture

As far as I understood this topics, problems mentioned there was because field value become incorrect after some action, be it Locale switch off, or upgrade from D6 with another database structure, than in D7. But problem with untranslatable Date field is reproducible on clean D7 installation with Entity_translation and Date modules. There are no legacy nodes, all necessary modules are enabled from the very beginning, and so on.

But I do believe you, of course, such bugs can be caused by extremely tricky dependencies.

das-peter’s picture

Just came across this issue and I really need a fix :) Debugging brought this result:

It looks like this is related to the function date_combo_value_callback.
This function is called by _form_builder_handle_input_element when it tries to set the value for an element.
If $element['#value'] isn't set, the function defined in '#value_callback' is called first - date defines date_combo_value_callback for it.
In the next step it's checked if a value was set for the element using !isset($element['#value']) - and that's exactly where the issue lies.
date_combo_value_callback returns an empty array if the parameter $input isn't set.
Unfortunately !isset($element['#value']) recognizes this empty array as valid content and thus $element['#value'] contains then an empty array instead the content of $element['#default_value'].

Later on form submit the empty array is posted and thus overwrites the actual value.
The attached patch changes date_combo_value_callback to return $element['#default_value'] if the parameter $input isn't set.

Besides that it contains an extendet check in date_combo_validate() to make sure the validation runs only if there's really something to validate.

The patch date-translating-deletes-date-field-entry-1178716-6-whitespace-fixes.patch fixes also some trailing whitespaces.

kdebaas’s picture

Status: Needs review » Needs work

Thanks for debugging this problem. I applied the patch, and can confirm that submitting a translation does not delete the date value anymore. The following error message is displayed, however
Notice: Undefined index: value2 en date_field_validate() (línea 304 de /home/klaas/web/example/modules/date/date.field.inc).

das-peter’s picture

kdebaas thanks for testing the patch. I couldn't reproduce the notice, but I've added some additional code to date_field_validate() to verify that value2 is available before accessing it.

kdebaas’s picture

On testing the new patch, no error notices are displayed anymore. Module now works as expected.

renat’s picture

@das-peter, thank you again!

With patch from #6 there really was problem, mentioned by kdebaas, with patch from #8 it is fixed. But there are one more problem still. If we use Date field with "Date's time zone handling", we'll receive this warning after translation submission:

Warning: DateTimeZone::__construct() [datetimezone.--construct]: Unknown or bad timezone (E) in DateObject->__construct() (line 103 of /var/www/test2.sky37.com/public_html/sites/all/modules/date/date_api/date_api.module).

When we'll open translated node, there will be such messages:

Warning: timezone_open() [function.timezone-open]: Unknown or bad timezone (E) in date_formatter_process() (line 313 of /var/www/test2.sky37.com/public_html/sites/all/modules/date/date.module).
Warning: date_timezone_set() expects parameter 2 to be DateTimeZone, boolean given in date_formatter_process() (line 313 of /var/www/test2.sky37.com/public_html/sites/all/modules/date/date.module).

With other time zone options everything is fine.

omercioglu’s picture

Patch seems fine, my date fields are intact now.

thtas’s picture

I'm also having this problem

Has this been committed to anything yet?

thtas’s picture

Okay i've used the patch mentioned in #8 against the latest 7.x.2.x (2011-09-30 - I had to apply it manually because dev has seen changes since that patch)

I have a from and to date set up.

So when i save my new translation, the to date is deleted, and the from date is set to the current day.

thtas’s picture

Status: Needs review » Needs work
drasgardian’s picture

subscribing

Pocketpain’s picture

sub

j0rd’s picture

There's a problem with how Date handles input. This has to do with the order of which is does things in form_building with $form_state['values'] ad $form_state['input'] . You can read more about this in http://api.drupal.org/api/drupal/includes--form.inc/function/form_builder/7

I'm having, what I believe is a related error when #access = FALSE is used with the Date module, since when #access = FALSE, $form_state['input'] is never populated. From looking at the patches provided, they seem to touch at the same code I was looking at when debugging.

#1304344: Date values cleared, instead of saved when updating a node when $form['myfield']['#access'] = FALSE

Pocketpain’s picture

Applied the patch #8, but now I get this error:

Notice: Undefined index: timezone i date_field_validate() (rad 290 av ../modules/date/date.field.inc).

The dates seems to be intact tho, but it still throws an error.

dboulet’s picture

Priority: Normal » Critical

Setting priority to critical, I just had all the dates on my site wiped out because of this bug.

I’ll test the patch in #8, hopefully this can be resolved soon.

dboulet’s picture

There is a duplicate issue in the Entity Translation queue here: #1186258: Entity translation deletes Date value, if Date field is untranslatable.

Still have to figure out if the bug is with ET or Date and mark one of these issues as a duplicate.

das-peter’s picture

Summary

In several scenarios the values of the date field are empty after saving an entity.
One issue seems to be that even if $form_state['input'] isn't available the script uses this non existent values to set the data in $form_state['values'] by using form_set_value();.
In all related issues there's the suggestion to replace $form_state['input'] by $form_state['values'] because this is also available in scenarios where $form_state['input'] isn't.
Since this issues are open a long time now without essential progress I decided to do exactly this and create a patch.
Besides that, I tag this issue as a D7 stable release blocker to go conform with webchicks request here: #1269708: Roadmap to create 7.x-2.0 release

Related Tickets:

Changes

I simply followed all the suggestions and made a search and replace action to replace $form_state['input'] by $form_state['values'] where applicable.

Patch

Would be great if this gets a broad review.
There are two patches - one without the trailing white-space fixes to make it easier to see where I really changed essential stuff.

mc-petry’s picture

patch #21 not works

still have a problem:

1) if date field is not translatable:
Notice: Undefined index: value2 in date_field_validate() (line 291 of ...\www\sites\all\modules\date\date.field.inc).

2) if date field set is translatable:
Notice: Undefined index: field_site_date in date_combo_validate() (line 407 of ...\www\sites\all\modules\date\date_elements.inc).

In both cases date field data deletes from database :(

das-peter’s picture

Updated patch according to mc-petrys feedback:

  1. Replaced this
    if (empty($item['value2']) && $item['value2'] !== 0) { with this
    if (!isset($item['value2'])) {
    If we use empty and it is true we don't know if the key value2 is set or not - accessing it can lead to a notice.
    isset on the other side also returns true if the value is 0 and thus is more suitable in this case.


  2. I made the check for existing input values more strict - I hope this helps

@mc-petry: Would be great if you could give this patch another try. Thanks.

drasgardian’s picture

The patch applied ok to the latest dev release of date.

Unfortunately though there was no noticable improvement. Dates were still lost upon creating a translation.

In my case the date field is not a translatable fields.

renat’s picture

Unfortunately, patch from #23 didn't solved mentioned problem for me. Translatable dates are ok when you create Entity translation - as they were, but untranslatable dates still vanishes - as they did before patching. D7.8, Date 7.x-2.x-dev (2011-Sep-08), Entity translation 7.x-1.x-dev (2011-Oct-11).

Anyway, thank you for all this efforts! Would be happy to test future patches.

mc-petry’s picture

thanks, das-peterfo #23 issue

if field translatable there is no errors, all works fine

but still have a problem with untranslatable fields

das-peter’s picture

Next round

If drilled down now to the root cause of this. I'm not sure if I've found the final solution, because it looks a bit hacky to me. On the other hand it seems to work as intended. ;)

Why?

On issue was that date_combo_value_callback() returned an empty array. This had the effect that no #value key for the form element was set.
Since the entity translation form fetches the fields by field_attach_form() and sets $form[$field_name]['#access'] = FALSE; on non translatable fields that are bad news for us. That's because the missing default value was stored later on in the database - and yes storing a missing value ends up in storing nothing. Unfortunately the form contained the necessary field structure to handle its data and thus we ended up explicitly store nothing aka. delete existing data.
Ok, with this change we've at least a default value.
But unfortunately the default value is already processed to be displayed in a form - means the timezone handling run already. This is an issue because on submit none of the date handlers is triggered to fix that because the element has '#access' = FALSE - this lack of processing leads to wrong values in the db.
Thus the next step was to modify date_combo_element_process(): Instead simply return the element on $element['#access'] == FALSE it takes now care of adjusting the values to be db ready.

Sugesstions? Any idea for a better solution would be very much appreciated! :)

Patches!

Today I've created three patches:

  • My regular one with removed trailing whitespaces
  • The -no-space-diff one - for which I used gits --ignore-space-at-eol
  • And last but not least the ultimate-whitespaces-fix - for this one I resaved all the code files to remove the trailing whitespaces as defined in the Coding Standards.
    If you use Eclipse I suggest to use the AnyEdit Plugin - this is able to remove trailing whitespaces automatically.

That's it - I hope we've now at least a workaround for this issue :)

dboulet’s picture

Status: Needs review » Needs work

Don’t know if the solution in #27 is the correct fix, but it does seem to work around the problem for me. Unfortunately it also results in tons of notices and warnings on the node translation page:

  • Notice: Undefined index: timezone in date_combo_element_process() (line 223 of /sites/default/modules/date/date_elements.inc).
  • Warning: DateTimeZone::__construct() [datetimezone.--construct]: Unknown or bad timezone () in date_combo_element_process() (line 223 of /sites/default/modules/date/date_elements.inc).
  • Notice: Undefined index: timezone_db in date_combo_element_process() (line 224 of /sites/default/modules/date/date_elements.inc).
  • Warning: DateTimeZone::__construct() [datetimezone.--construct]: Unknown or bad timezone () in date_combo_element_process() (line 224 of /sites/default/modules/date/date_elements.inc).
  • Warning: DateTime::setTimezone() expects parameter 1 to be DateTimeZone, null given in DateObject->setTimezone() (line 225 of /sites/default/modules/date/date_api/date_api.module).
  • Notice: Undefined index: date_type in date_combo_element_process() (line 225 of /sites/default/modules/date/date_elements.inc).
  • Notice: Undefined index: timezone in date_combo_element_process() (line 223 of /sites/default/modules/date/date_elements.inc).
  • Warning: DateTimeZone::__construct() [datetimezone.--construct]: Unknown or bad timezone () in date_combo_element_process() (line 223 of /sites/default/modules/date/date_elements.inc).
  • Notice: Undefined index: timezone_db in date_combo_element_process() (line 224 of /sites/default/modules/date/date_elements.inc).
  • Warning: DateTimeZone::__construct() [datetimezone.--construct]: Unknown or bad timezone () in date_combo_element_process() (line 224 of /sites/default/modules/date/date_elements.inc).
  • Warning: DateTime::setTimezone() expects parameter 1 to be DateTimeZone, null given in DateObject->setTimezone() (line 225 of /sites/default/modules/date/date_api/date_api.module).
  • Notice: Undefined index: date_type in date_combo_element_process() (line 225 of /sites/default/modules/date/date_elements.inc).
das-peter’s picture

@dboulet: Please specify your date-field configuration otherwise it's hard to try to reproduce this behaviour.

drasgardian’s picture

The errors reported by @dboulet in#27 occur when a date field is NOT set to 'Collect an End Date'. It is throwing an error when trying to load the second value.

I was able to fix this by changing

foreach (array('value', 'value2') as $value_key) {

to

foreach ($element['#columns'] as $value_key) {

I have also attached a new patch file, it's just the first patch from #27 with this one line changed.

@das-peter perhaps a separate issue addressing the excess whitespace in the module should be opened?

dboulet’s picture

Actually, that is not the case. Here is the setup for my field:

Number of values: Unlimited
Date attributes to collect: Year, Month, Day, Hour, Minute
Collect and end date: Checked
End date required: Not checked
Time zone handling: Site’s time zone

The field is a regular date field on the Article content type, it is not set to be translated by users. It is not set to be required and it uses the date popup widget.

drasgardian’s picture

@dboulet, can you test with 'collect an end date' NOT checked?

My setup has two Date (ISO format) fields, one collects an end date and the other doesn't. Both collect Year, Month and Day values only, which means there is no timezone handling.

dboulet’s picture

Ok, I get even more warnings with “Collect an end date” not checked, a page full of:

Warning: DateTime::format() [datetime.format]: The DateTime object has not been correctly initialized by its constructor in DateObject->format() (line 241 of /sites/default/modules/date/date_api/date_api.module).

dboulet’s picture

Status: Needs work » Needs review
StatusFileSize
new5.16 KB

Added an if statement to get rid of warnings.

dboulet’s picture

Status: Needs review » Needs work

Ok, testing out this last patch some more and it doesn’t work when the date field is required. The date values still get erased.

renat’s picture

Unfortunately, patch from #30 doesn't play well with Date's field "Time zone handling" set to "Date's time zone". At translation page (node/NID/translate/add/ru/en) we do have such a warning:

Warning: DateTimeZone::__construct() [datetimezone.--construct]: Unknown or bad timezone () in date_combo_element_process() (line 223 of /var/www/sky37.pp.ua/public_html/sites/all/modules/date/date_elements.inc).

When we try to save translation, we'll receive this:

    Warning: DateTimeZone::__construct() [datetimezone.--construct]: Unknown or bad timezone () in date_combo_element_process() (line 223 of /var/www/sky37.pp.ua/public_html/sites/all/modules/date/date_elements.inc).
    Warning: DateTimeZone::__construct() [datetimezone.--construct]: Unknown or bad timezone (E) in DateObject->__construct() (line 103 of /var/www/sky37.pp.ua/public_html/sites/all/modules/date/date_api/date_api.module).
    PDOException: SQLSTATE[01000]: Warning: 1265 Data truncated for column 'field_datetest20_offset' at row 1: INSERT INTO {field_data_field_datetest20} (entity_type, entity_id, revision_id, bundle, delta, language, field_datetest20_value, field_datetest20_timezone, field_datetest20_offset) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7, :db_insert_placeholder_8); Array ( [:db_insert_placeholder_0] => node [:db_insert_placeholder_1] => 187 [:db_insert_placeholder_2] => 245 [:db_insert_placeholder_3] => testtype [:db_insert_placeholder_4] => 0 [:db_insert_placeholder_5] => und [:db_insert_placeholder_6] => 2011-11-02 23:00:00 [:db_insert_placeholder_7] => E [:db_insert_placeholder_8] => 1970-01-01 04:00:00 ) in field_sql_storage_field_storage_write() (line 448 of /var/www/sky37.pp.ua/public_html/modules/field/modules/field_sql_storage/field_sql_storage.module).
das-peter’s picture

Status: Needs work » Needs review
StatusFileSize
new5.35 KB

Here we go, next round.
I was able to reproduce and fix the timezone issue.
Now let's find and fix the next issue ;)

@drasgardian: I've just created an separate issue: #1330768: Code cleanup: Remove trailing white-spaces

renat’s picture

Wow, looks like you nearly did it! With latest patch I managed to find only one minor bug, also with timezone. In case you are trying to translate node with empty Date field ("Date's time zone" mood, of course), you'll receive such a notice:

    Notice: Undefined index: timezone in date_combo_element_process() (line 224 of /var/www/sky37.pp.ua/public_html/sites/all/modules/date/date_elements.inc).
    Warning: DateTimeZone::__construct() [datetimezone.--construct]: Unknown or bad timezone () in date_combo_element_process() (line 224 of /var/www/sky37.pp.ua/public_html/sites/all/modules/date/date_elements.inc).
    Notice: Undefined index: timezone_db in date_combo_element_process() (line 225 of /var/www/sky37.pp.ua/public_html/sites/all/modules/date/date_elements.inc).
    Warning: DateTimeZone::__construct() [datetimezone.--construct]: Unknown or bad timezone () in date_combo_element_process() (line 225 of /var/www/sky37.pp.ua/public_html/sites/all/modules/date/date_elements.inc).
    Warning: DateTime::setTimezone() expects parameter 1 to be DateTimeZone, null given in DateObject->setTimezone() (line 231 of /var/www/sky37.pp.ua/public_html/sites/all/modules/date/date_api/date_api.module).
    Notice: Undefined index: date_type in date_combo_element_process() (line 226 of /var/www/sky37.pp.ua/public_html/sites/all/modules/date/date_elements.inc).
    Notice: Undefined index: timezone in date_combo_element_process() (line 224 of /var/www/sky37.pp.ua/public_html/sites/all/modules/date/date_elements.inc).
    Warning: DateTimeZone::__construct() [datetimezone.--construct]: Unknown or bad timezone () in date_combo_element_process() (line 224 of /var/www/sky37.pp.ua/public_html/sites/all/modules/date/date_elements.inc).
    Notice: Undefined index: timezone_db in date_combo_element_process() (line 225 of /var/www/sky37.pp.ua/public_html/sites/all/modules/date/date_elements.inc).
    Warning: DateTimeZone::__construct() [datetimezone.--construct]: Unknown or bad timezone () in date_combo_element_process() (line 225 of /var/www/sky37.pp.ua/public_html/sites/all/modules/date/date_elements.inc).
    Warning: DateTime::setTimezone() expects parameter 1 to be DateTimeZone, null given in DateObject->setTimezone() (line 225 of /var/www/sky37.pp.ua/public_html/sites/all/modules/date/date_api/date_api.module).
    Notice: Undefined index: date_type in date_combo_element_process() (line 226 of /var/www/sky37.pp.ua/public_html/sites/all/modules/date/date_elements.inc).

However, even in this case data seems OK, so it's a huge progress.

das-peter’s picture

Damn, this issues drives me crazy :|
The longer I try to fix the issues the more it looks like the whole form handling needs some serious refactoring.

Next patch attached - had to get rid of validators and make sure the field stays empty if it has to be empty.

renat’s picture

I'm happy to say, that after extensive testing (all Date's field types, all widgets, all Timezone handling variants, with and without end date and date itself, with two Date field in one node, manual and programmatic translation creation) I've found no bugs with patch from #39!

But having in mind extreme complexity of this issue I'll not mark it as RTBC yet, hope there will be at least one more tester. If not, I'm going to mark it as RTBC till 3 days, after some usage of patched module on production site.

das-peter’s picture

@renat Thanks for testing. One thing I didn't test is Date repeat. And I'm really afraid of this... :D

jherencia’s picture

I have tested the patch in my current environment (no date repeat and no date translation) and it works perfect. Thanks for such a great work on coding and reviewing.

renat’s picture

Status: Needs review » Needs work

Unfortunately, there really is a problem with Date repeat. Without patch it works, and with patch we do see "The UNTIL value is required for repeating dates" message, when we are trying to save node, whether UNTIL is filled or not.

Quick search shows us, that we are not alone with this problem, it appeared yet, even in D6 version of this module, though I don't think they are exactly the same.

das-peter’s picture

Status: Needs work » Needs review
StatusFileSize
new7.3 KB

I knew it - it would have been to nice that it simply works ;)
So, next round - this time with additional changes to get date_repeat working too.

renat’s picture

Status: Needs review » Needs work

And now node actually saves, and data seems ok:

[field_datetest37] => Array
(
[und] => Array
(
[0] => Array
(
[value] => 2011-11-05 10:15:00
[timezone] => Europe/Moscow
[offset] => 0
[rrule] => RRULE:FREQ=DAILY;INTERVAL=14;UNTIL=20120119T200000Z;WKST=MO
[timezone_db] => UTC
[date_type] => datetime
)

[1] => Array
(
[value] => 2011-11-19 10:15:00
[timezone] => Europe/Moscow
[offset] => 0
[rrule] => RRULE:FREQ=DAILY;INTERVAL=14;UNTIL=20120119T200000Z;WKST=MO
[timezone_db] => UTC
[date_type] => datetime
)

[2] => Array
(
[value] => 2011-12-03 10:15:00
[timezone] => Europe/Moscow
[offset] => 0
[rrule] => RRULE:FREQ=DAILY;INTERVAL=14;UNTIL=20120119T200000Z;WKST=MO
[timezone_db] => UTC
[date_type] => datetime
)
... and so on ...

But when we create translation (manually, there are no problems with programmatic creation), all repeating dates vanishes, except first:

[field_datetest37] => Array
(
[und] => Array
(
[0] => Array
(
[value] => 2011-11-05 10:15:00
[timezone] => Europe/Moscow
[offset] => 0
[rrule] => RRULE:FREQ=DAILY;INTERVAL=14;UNTIL=20120119T200000Z;WKST=MO
[timezone_db] => UTC
[date_type] => datetime
)

)

)

das-peter’s picture

Status: Needs work » Needs review
StatusFileSize
new7.3 KB

After another nerve-wrecking debug session here it comes, the next patch.
Unfortunately the longer I hack on this the more I'm in doubt I use the right approach.
But in lack of an alternative I keep going in this direction.

@renat: Thank you very much for the extensive testing - it's really a great help!

renat’s picture

Unfortunately, I wasn't able to notice any changes with patch from #46, all repeating dates still vanishes, except first.

And, of course, I'm glad to help with this issue, because, from my point of view, it is extremely important to make Entity translation work. First, it gives us all-new impressive possibilities (without it it was de-facto impossible to build complex multilingual information systems with Drupal). Second, it is a basis for all Drupal 8 Multilingual initiative, and if there will be no popular "proof-of-concept" contrib D7 module, we can easily have half-baked solution in core.

das-peter’s picture

*grml* looks like I was really somehow confused and posted the same patch twice.
Here comes the new one :)

@renat: Same here, I think entity_translation is the future - thus let's make it future proof ;)

renat’s picture

Status: Needs review » Needs work

Patch from #48 solved mentioned problem, but introduced a new one. In case we have a node type with Image field (default settings, untranslatable) and with Date fields (Date repeat, other settings are default, untranslatable), we can't add node translation manually - it will not be saved, and we'll see such message in our log:
PHP Fatal error: Call to undefined function date_repeat_build_dates() in /var/www/sky37.pp.ua/public_html/sites/all/modules/date/date_repeat/date_repeat_form.inc on line 286, referer: http://sky37.pp.ua/en/node/186/translate/add/fr/en?render=overlay

And sorry for a late response, was a bit tricky to realize the reason this bug occurs.

karens’s picture

I committed the whitespace issue patch so we can keep the whitespace out of this issue.

See #1250172: $form_state['input'] vs $form_state['values'] usage breaks default values about the 'input' vs 'values'. I think it would be cleaner to separate that issue out and deal with it first, then come back here and see what remains to be fixed.

There's also another issue about translation problems at #1057544: Date field language handling problems. Not sure if some of the solution is in there or not, just want to get them linked.

das-peter’s picture

@KarenS: Thanks for the statement. I'll check which of the 'input' / 'values' changes could be removed from this patch. Next time-frame I see in which I can continue to work on this patch is in about 6 hours.

I think we're close on having a working workaround - but frankly speaking I'm not confident that this is a sustainable solution. However, a deeper refactoring seems to be out of scope atm.
Other impressions / suggestions?

das-peter’s picture

Status: Needs work » Needs review
StatusFileSize
new10.26 KB

@renat: I wasn't able to reproduce the error. I've added an image field instance and also recreated a new date repeat field instance with the default settings set. Even changing to different options on the date field (widget, timezone usw.) didn't help to trigger an error as you described it.
However, since the error is very specific in what caused it I've added a module_load_include to make sure the necessary file is loaded. If that doesn't help something other seems to be broken badly ;)

renat’s picture

Status: Needs review » Needs work

From my point of view, we should complete patch, created in this theme - in case it's committing will not make later work much more difficult. The reason is simple, this is too important problem, leading to user's data loss. It would be nice, of course, to do things in the right way from the very beginning, but who knows, how long will it take to fix "$form_state['input'] vs $form_state['values'] usage breaks default values" issue?

As of Date field language handling problems, I use Views filtration for nodes with translatable Date field, and it is possible to limit that dates. I faced no problems with it, though I can't say there can be no problems in principle, because I do not see exact steps to reproduce claimed issue in the mentioned topic, so can't check it.

And, of course, it would be nice, if Date field will not became untranslatable, as it was proposed in that topic. "Translatable" may sounds a bit confusing, but it simply means, that we can enter different values for different languages, and that makes a lot of sense in some cases.

renat’s picture

Status: Needs work » Needs review
mc-petry’s picture

oh thanks das-peter
last patch 52 work fine!

renat’s picture

Confirm, patch from #52 solved my problems, looks like everything is fine now. But a bit later today I'll make extensive test to be absolutely sure there are no more side effects.

karens’s picture

I'm taking this apart and trying bits at a time to be sure other things aren't broken in the process. The first commit is for:

 function date_combo_value_callback($element, $input = FALSE, &$form_state) {
   if (!$input) {
-    return array();
+    return NULL;
   }
 }

I have created a site using translations and can replicate the date getting lost on an untranslatable date field and confirmed that the change doesn't seem to break anything else and that the tests still work. This change alone seems to fix that problem.

renat’s picture

Tested patch from #52 once again (Date repeat, all Date's field types, all widgets, all Timezone handling variants, with and without end date and date itself, with two Date field in one node, manual and programmatic translation creation) - and now everything is fine. Hope there will be no problems for @KarenS during committing process as well.

Congratulations, guys, and, @das-peter, thank you!

das-peter’s picture

That are really great news :)

@KarenS: #57 was indeed a killer for a lot of functionality. If I understand this right we can split the basic problem into two parts:

1.The value callback which caused that no proper default value was set.
It could actually be that because of this issues other "workarounds" were necessary the get the form elements running. And thus lead to some special constructs.
2. Setting the values that way they can be stored directly back to the db.
It's likely I made mistakes in the patch in this part.
The idea I had here for a possible refactoring was to keep the unprocessed values, the ones to store right away in the db, in a form container of the type value. If the date elements are accessible the value container is adjusted by the post-processing functions of the date elements on submit. If the elements aren't accessible the whole processing part is simply skipped.
karens’s picture

@das-peter I need to break this issue down into its individual components, it has become a mash of lots of different problems and the solutions for each. I am trying to re-create the problems one at a time and fix them. It's not always clear exactly what problems the different parts of the patch are trying to fix. If there are two remaining 'problems', can you describe exactly how I can reproduce them so I can see for myself what is going on?

I don't have a real international site. I have just set up a site that has a lot of date fields and am trying to reproduce the issues there. So as a part of the problem description I need to know what modules are enabled and generally how the multilingual/translation settings are configured so I can match it. For instance, I figured out that the first problem appears to happen if only Local and Entity Translation are enabled, a content type is set up to be translated but the date field is NOT translated.

dboulet’s picture

I’ve been testing the patch in #52 and it does seem to work well. I tried different combinations of date granularity, having an end date/no end date, and having the field be required/not required—couldn’t find any problems with saving node translations while having the date field being non-translatable.

@das-peter and @KarenS, thank you very much for all you efforts, it is much appreciated.

karens’s picture

Notice: Undefined index: value2 in date_field_validate() (line 291 of ...\www\sites\all\modules\date\date.field.inc) is a separate issue, unrelated to translation. I'm going to open a new issue for that and we should keep it out of this thread. That happens (I think) any time you have a date with an optional end date and don't supply one. The logic handling this is wrong and needs to be fixed, but that is a different issue.

karens’s picture

I just committed a couple other fixes for things that are contributing to this problem:

- Fix date repeat form values that are not arrays when hidden on a node that has translation.
- Tweak the date repeat widget to identify empty input when used on nodes with translation.

karens’s picture

Got a few more individual issues fixed:

- Default value callback for the timezone widget was not returning an array.
- Removed value_callback for date_repeat and date_combo forms, the default behavior works fine.
- Use drupal_array_get_nested_value() in Date Repeat instead of trying to find it manually.

Testing the patch above won't help any more. I am picking out individual items and committing some of them, skipping some, and adding some. There are a few places where the patch is not doing exactly what we need and others where we need some additional fixes.

A huge thanks to das-peter who emailed me a tarball I could use to debug these issues. It had an install profile and feature that created a translation site that has the kinds of fields he was seeing problems in, with easy instructions on how I could spin it up using Drush. He is the most amazingly helpful bug reporter I have ever seen :)

das-peter’s picture

*blush* Thank you for these credits - I've to say it's a pleasure to be able to contribute. Even more if I got such nice feedback!

Looks the thing I was afraid of came true - I didn't have the appropriate knowledge about the internals of the module to do more as a hackish workaround.
Thus an extra "thank you" to Karen for tracking down the root of each single symptom again.

Btw. should we write tests for that? On the complexity of this module that could be really a life-saver in future.

karens’s picture

We should absolutely write tests for this. Are you up for the challenge? I need to focus on fixing the rest of the problems.

karens’s picture

Several more problems have been fixed now. I think we are getting pretty close to having it work correctly.

- By pass date repeat widget processing and validation when element is hidden from user.
- Don't do timezone adjustments in the widget, wait for #process so we can skip it when the date field has been hidden by #access.
- Logic for creating end date wasn't taking into account the possibility that a field might have no value2.

karens’s picture

Status: Needs review » Fixed

A few more commits:

- Switch drupal_array_get_nested_value() to use 'values' instead of 'input', where it makes sense, and simplify some of this code.
- Create a helper function for testing hidden/disabled dates and test each date element and validator to skip processing in that case.
- When hidden by #access=FALSE, repeating date fields were getting removed and not replaced.

With those and the ones I previously committed, I'm going to call this fixed. It looks to me like things are working correctly.

karens’s picture

Component: Code » Translation

Flagging the translation issues.

brycesenz’s picture

Component: Translation » Code
Status: Fixed » Needs review

Confirming that the patch in #52 fixed my issue.

colan’s picture

Component: Code » Translation
Status: Needs review » Fixed

Please don't change the status unnecessarily.

karens’s picture

The patch in #52 was not used. Individual issues were committed, some the same as the patch, some different, as noted in all the comments above. And all the changes are committed so there is no need to patch anything anyway.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

sreese’s picture

Version: 7.x-2.x-dev » 7.x-2.1
Status: Closed (fixed) » Needs work

I'm not sure if this is the right place for me to post but it appears to me that this issue is not yet resolved. My problem most closely resembles the one outlined in #1267434: Dates fields set to #access=FALSE are emptied on save but a note in that issue says this is the appropriate issue to post about it so here I am.

I have a node creation form that I converted into a multistep form via a custom module using hook_form_FORM_ID_alter. It works by setting #access to FALSE if you don't want the field to show up on the current step. I can confirm that the date field does not preserve any data that has been entered when creating a new node. The odd thing is if I edit the node and re-enter the date, the date is then preserved and saved across all the steps. It would appear the problem may be solved when saving existing nodes but it is apparently not resolved when creating new nodes.

I used the devel module to get a look at what was happening and discovered that in the node/add form, the attribute $my_date_field['und'][0]['#date_is_default'] is being set to TRUE right off the bat which seems to mean this field will be set to the default value no matter what the user fills in. Upon editing the node, #date_is_default is set to FALSE which seems to allow setting the date normally.

I can confirm that by commenting out the following code from date_elements.inc, that the date is stored normally and saved with the node:

  $is_default = FALSE;
  if (!empty($instance['widget']['is_new'])) {
    $items = date_default_value($field, $instance, $langcode);
    $is_default = TRUE;
  }

I guess what is happening here is that the date that was entered in step 1 on the form gets overwritten by the default value on step 2 of the form. I could just comment this code out and leave it that way but I thought I'd post here to see if perhaps there was a more elegant solution that might help future users of this module who might want to inclue a date field in a multistep form. I would be happy to provide any information you might find useful in resolving this issue and I will happily test any possible workarounds, patches, etc.

karens’s picture

Status: Needs work » Closed (fixed)

A problem with a default value in a multistep form is a totally different issue than this one. And there is already an issue about multistep forms.

sreese’s picture

I searched but did not see an open issue for this. The one I linked to was closed. Can you point me at the current open issue? I am unable to find it by searching for 'multistep' so maybe I'm just not looking in the right place?

sreese’s picture

StatusFileSize
new687 bytes

For anyone else who might be interested in this particular issue, I have found a fix that seems to work. I added an additional check to the is_default 'if' statement to see if a value has been set for the field yet. If not, assign the default value. If so, keep the value that's in there. This works for my purposes and fixes the multistep issue for me but I'm not sure if this will work for others' use cases or not.

I've attached a patch in case anyone wants to check it out. Cheers.