Convert language:weight into a protected property

First attempt, the possibility that I have missed something is high, but the fix should be obvious.

Please note the isset line in the getWeight() function.

Ahem the problem is simple .. if only I had seen it first time..

Postponing on resolution of #2304651: Core Language sort() should not work, needs to use name instead of default title

I am opening up this issue again.... it look like [ #2304651] may have solved the random fail issue.

In the long term the plan is :-
Apply its fix to all issues, and the EntityQueryTests should go green that will provide some confidence, BUT the next step,
is to run version of the patch 50 times over in #2254011: Drupal\system\Tests\Entity\EntityQueryTest random failure

I am just folding in the change here ... this issue has other problems to fix first.

Restored sort line to its original :-

Language::sort($this->languages);

Drupal\language\Tests\LanguageConfigurationTest now passes locally.

@tstoeckler

I agree in part ....

Can I rephrase the question

What is to stop non-integer or NULL values being passed into the constructor or the setter?

The default values are made available for another programmer, but its use is not tightly enforced. There is no type checking. and nothing to stop the construction of a language with a weight not set ( ie set to null ). This will cause difficult to see issue in the long run.

What goes for weight, can also be said of locked ( https://www.drupal.org/node/2226533#comment-8942261 )

I have made the case in the past, in another issue, for these values to made-safe at point to intake. ( in setters or constructors )

But I was going for the simplest solution here, well easiest to review ... we have a maintenance problem at the moment as
this "ternary operator" checking is made at two places in core and it seemed natural to consolidate the fixup in the getter.

Anyway I would like to hear what you think?

To cut away at my own argument

$language_entity->weight = isset($language->weight) ? $language->weight : 0;

occurs at only ONE place language_save :-

  $language_entity->label = isset($language->name) ? $language->name : '';
  $language_entity->direction = isset($language->direction) ? $language->direction : '0';
  $language_entity->locked = !empty($language->locked);
  $language_entity->weight = isset($language->weight) ? $language->weight : 0;
  $language_entity->setDefault(!empty($language->default));
  $language_entity->save();

Other properties may be typed check in multiple places ( I am looking at you locked ) but not weight.

So I am returning getWeight() to the more consistent form..
[ I cannot use isset(func()) so I am using null !== func() instead. Its is PHP thing.]

It is a truism that many of my concerns about loose type checking in core could be resolved by providing better documentation above the constructor...
BUT even I think this would be overkill in this case ... using Language::defaultValues is a specialist thing that not many developers down the line
are going to want to do and for those that do there are plenty of examples in core for people to copy..

-----------------------------

I have fixed a minor nit pick in EntityTranslationTest but it is not the complete story, errors persist.

I will look at this again soon, but maybe not until the weekend

I am postponing because I am hopeful, that #2226533: Changes to the Language class due to the LanguageInterface (followup) is on its way to being committed.

Re-opened in light of https://www.drupal.org/node/2226533#comment-9023095

Patch no longer applied. Reroll.

EntityTranslationTest can be made to pass by changing the definition of Drupal\Core\Language\Language::weight from
protected to public,

So it looks like not all code of the form language->weight has been changed to language->getWeight()

I have looked but cannot find it. :(

Needed reroll.

The original intent behind this issue is still valid.

#2226533: Changes to the Language class due to the LanguageInterface (followup) is too large and too cumbersome to get reviewed... so splitting off small segments that are obviously right, and can be reviewed faster.

Rerolling is complete .. NowI have finally fixed the failing tests by copying over changes from the parent issue.

Reroll triggered by

#2332739: Remove SortArray::sortByWeightAndTitle

as is to be expected a file diff between #28 and #31 shows only a simplification of Drupal\Core\Language\Language::sort()

PS I am removing the will case commit conflicts tags .. it was inherited from its parent which typically needs a daily reroll.. This issue is much smaller and has gone for weeks without a reroll...

Patch no longer applies.

@YestCT I will remove that ternary ... later ( tonight )

Just to cross reference, this other active issue, which I think I agree with.. aims to remove the setWeight()

So in the future the patch here may become even smaller.

Just a straight reroll, for now.

one conflict which was trivial to resolve.

Snip snip ..now using $language->getWeight() without protection.

Needs a reroll... there is a sprint so I am just signalling that I am working on this now.

patch is simpler because common.inc langauge_save() has gone (Yay!)

Hurumph

I could only see one place to alter, other than the definition itself.

I guess testbot will give me a better answer.

LanguageConfigurationTest now passes.