๐Ÿ‡ฎ๐Ÿ‡ณIndia @omkar.podey

Account created on 13 August 2021, almost 3 years ago
  • Associate Engineer at Acquiaย 
#

Merge Requests

More

Recent comments

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

Right now \Drupal\FunctionalTests\Core\Recipe\StandardRecipeInstallTest::testStandard is failing and I think it is because of the changes in core/modules/system/config/schema/system.schema.yml as we have a empty profile in the test and stark theme isn't installed.

So what should we do here ?, just install stark at the start (which seems difficult without the container) and i was wondering would we encounter a similar problem while using recipes on a real site using the empty profile.

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

omkar.podey โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

omkar.podey โ†’ made their first commit to this issueโ€™s fork.

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

I would like to get it reviewed first and then we can decide if we need the 11.x branch explicitly.

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

omkar.podey โ†’ changed the visibility of the branch 3436632-add-validation-constraints to hidden.

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

Okay so I switched to 10.3.x. and right now the only test that's failing is Drupal\Tests\user\Kernel\UserAdminSettingsFormTest::testConfigForm

To fix this there were two options, either add config target data to core/modules/user/src/AccountSettingsForm.php or mark the keys that are failing the test as requiredKey: false in user.schema.yml.

So I thought that the requiredKey false would be the solution but then I ran into the langcode issue again where the langcode isn't defined in user.schema.yml so I can't mark it requiredKey: false.

thoughts ?

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

omkar.podey โ†’ changed the visibility of the branch 3436164-add-validation-constraints to hidden.

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

I simplified the whole code block by removing the creation of new field storage config and hence moving away from the whole translatable key issue.

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

The migration tests and UserMailNotifyTest isn't happy with making all the keys required because of marking user.settings as fullyValidatable .I could add keys to UserMailNotifyTest and remove the requiredkey: false from most of them, doing that next.

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

While that might be true that the langcode shouldn't be added. I think it's a problem for another day, without the langcode being set now this won't pass tests and not be committed at all should we really block this on that?

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

It is related but i think this landing independently shouldn't be a problem

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

cancel_method has to be nullable according to testConfigForm

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey
  1. register key could have 3 values according to \Drupal\user\UserInterface
  2. cancel_method can have any function assigned to it but i think it can't be empty.
  3. password_reset_timeout It is never being set just being read so I think it should just not be null.
๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

Since there is no defined upper limit it makes sense to just add a lower limit for the default value and that it's not null I think.

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

Right now in the 11.x branch following the default_content module's convention which is to place the .yml file in the entity_type folder under the recipes folder works. You can try that by running php core/scripts/drupal recipe core/tests/fixtures/recipes/only_content.

  1. Needs cleaning next as multiple classes are added which could be combined.
  2. Needs implementation for handling media files.
๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

@bnjmnm could you approve that the aria-label in div looks good.

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

Even if we strip off all the normalise stuff we still need the denormalise for yaml content files right ?, this means a new class for denormalise as it's a lot of code .

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

The latest changes look good, @alexpott if you can approve this too that would be great.

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

Just to clarify this โœจ Use modals in field creation and field edit flow Needs work isn't going to go in at all the issue had a lot changes at once and hence it was split up into smaller pieces that could go in independently.

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

I guess the show password isn't that important on the login page so just removed it with the strength indicator.

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

Split into controller and form. So we have no forms without form elements.

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

Set to needs work for writing a better test.

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

omkar.podey โ†’ changed the visibility of the branch 2663316-broken-title-in to active.

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

omkar.podey โ†’ changed the visibility of the branch 2663316-broken-title-in to hidden.

๐Ÿ‡ฎ๐Ÿ‡ณIndia omkar.podey

Updated descriptions, more suggestions ?

Production build 0.69.0 2024