Merged. Will set up a new release soon.
I assume this worked. Please feel free to reopen if not. Right now, the module installs fine on Drupal 9.5 at least (and soon on Drupal 10).
I just committed a few changes related to timezones in 📌 Handle timezones Fixed and that might be tangentially related. Can you try this again?
A fresh test with Drupal 9 and 10 showed that timezones are not handled correctly. When saving a "Date only" field, the actual date written to the database is a day earlier (as I am in Eastern timezone and Drupal converts the date to UTC). For example, today is 4th Jumadil Ula and 16th November; saving this date will store 7 PM 15th November. The time is discarded and only 15th November is stored. On the next load of the edit page, it will convert 15th November to 3rd Jumadil Ula. Saving this will store 14th November and the drift repeats.
The problem is that the date form element we have handles the date conversion and it returns a string after formatting the timestamp with Drupal API. Of course, the timezone conversion happens here. Therefore, the date form element (TaarikhDate.php) should set the timezone before returning the formatted string.
Updated README with instructions.
We use DDEV to work on this module. The DDEV configuration is ignored from git for two reasons:
1. It's unnecessary.
2. It locks in the Drupal version.
Instead, we'll use a script to set up the development environment. Fortunately, [ddev-drupal-contrib](https://github.com/ddev/ddev-drupal-contrib) makes this very easy for us. The steps mentioned there are scripted in `./setup_dev.sh` and some more. The script also lets you specify the Drupal version to use (corresponds to DDEV recipes: `drupal8`, `drupal9`, and `drupal10` as of right now). It also copies a settings.local.php and modifies settings.php to have it take effect.
Usage:
./setup_dev.sh drupal9
This will set up a DDEV site at the address https://taarikh-drupal9.ddev.site (if you have cloned this repo in a directory called `taarikh`). Now you can install the site and happy testing.
hussainweb → created an issue.
This has been fixed and tested for a while on PHP 8. Marking this as fixed.
hussainweb → created an issue.
It turned out to be simpler than I thought. Merging and fixed.
hussainweb → created an issue.
The phpunit task is still failing and I'll do this in a follow-up (even phpstan). For now, this is merged.
There is an error: 'Could not open input file: vendor/bin/phpstan'. This seems to be a problem with how Gitlab CI is configured in templates. There is some information in https://www.drupal.org/project/gitlab_templates/issues/3395419 → but it seems that this was overlooked. Possible fix is that we include phpstan as a dev dependency which should be done in a different issue. For now, we are good to ignore this.
Thanks! I'll test it today and commit.
hussainweb → created an issue.
Fixed and all jobs in the Gitlab CI pipeline now pass.
hussainweb → created an issue.
hussainweb → created an issue.
For the second error, it is a common Drupal practice and the advice is to ignore it as per https://www.drupal.org/docs/develop/development-tools/phpstan/handling-u... → . The other two errors are fixed.
The module contains a phpcs.xml.dist from here: https://git.drupalcode.org/project/examples/blob/HEAD/phpcs.xml.dist.
As such, the current version of the file is very similar to the one in the above link (except the sniff referred to in the error message). While we can update the file, I think it is more productive to just remove the file and let Gitlab CI pull in the default one. We can bring this one back if we need to customize anything.
hussainweb → created an issue.
hussainweb → created an issue.
As it turns out, the file being reported is drupalci.yml and we don't need that anymore. Deleting it...
hussainweb → created an issue.
Committed and pushed. Thanks.
This seems to be a recent change to TypedData interface as I have not seen this before. Thanks for the patch. I can see that the same pattern in a variety of classes in Drupal core.
I released a new version of a dependency package that solves the problem for `core-recommended`. I then tested this on Simplytest and this module is installable on Drupal 10. No further change was needed in the module itself.
I tried this again today and this still needs work. The module is installable on Drupal 10 if you use `Drupal/core` composer package. But not installable if you use `Drupal/core-recommended`.
Thanks for your work but I want to make a release today and so I am just going to push a simple fix for now. We can consider autowiring later. If you're interested in taking this further, please create an issue.
Committed and pushed. Thanks!
Thanks, @Shreya_th. I am afraid this doesn't work either. You are reading the expression into a variable and the warning is generated at that point. By the time you check for `!empty`, the warning is already generated.
The fix is to use the null-coalescing operator.
Committed, thanks!
Patch in #4 is good. I am afraid that MR !7 is targeting the wrong branch.
IMHO, we can just fix the error here. Autowiring and testing it is a larger problem we can worry about later. I am not sure that TypedData even supports injection this way. Of course, I wrote this ages ago when autowiring was not implemented in Drupal core, so things must have changed.
The fix is already there in your MR. You just need to remove the injection bits and we can create another issue to inject the service properly.
@abhishek_virasat, I think you're trying to rely on autowiring for this to work, but autowiring is not enabled. Did you maybe forget to commit that change? Or am I missing something here?
@jagraj_singh_gill, your change is good in theory but we should return an empty string rather than `NULL`. This value gets used within Drupal and I am worried this NULL will throw other errors. We only hit this case when the bio is empty on Drupal.org. In that case, empty string is good enough. Please make this change and I can commit.
Thanks for the fixes.
@Shreya_th, I see your patch and MR are different. The patch has accidentally changed the indentation.
+++ b/src/DOComputedFields.php
@@ -33,42 +33,59 @@ class DOComputedFields extends TypedData {
+ return is_array($userInformation->field_bio) && isset($userInformation->field_bio[0]['value']) ? $userInformation->field_bio[0]['value'] : 'N/A';
I see you are treating the `$userInformation` as a Drupal field value. It is not. It is a simple PHP class and the `field_bio` (magic) property is not an array (unless it is empty). So the above code will always return "N/A". Yes, it won't throw an error.
In your MR, on the other hand, you are using a function called `get` but that function doesn't exist on the `User` class at all (nor on the base class).
hussainweb → created an issue.
I accidentally created the same issue for 2.x at 📌 Decide if we should accept actual username or URL slugs Active . Closing this one.
hussainweb → created an issue. See original summary → .
hussainweb → created an issue.
@apaderno, like I said in #3, the patch/commit is not visible here but there is still something that needs to be tested, and hence it is set to "Needs Review". From the documentation → :
Note that if a patch is not required in order to complete the idea described in the issue, then it is the idea itself that needs review.
I am interpreting that as there is something here that needs to be reviewed and it is fine for the field to be set that way. I understand it might break your workflow and I don't know of any solution for that.
Just removing the $logger property on the class is sufficient. That property is made available by LoggerAwareTrait anyway and the error was due to different types (it needed to be `?LoggerInterface` whereas it was missing and assumed `mixed` in the module's class).
We can still set to the property in the constructor because LoggerInterface is compatible with Drupal's `\Drupal\Core\Logger\LoggerChannelInterface`.
hussainweb → created an issue.
hussainweb → created an issue.
@abhishek_virasat, did you try recently or are you saying that you were able to install this before I created this issue? I have already fixed this issue but for some reason, the commit doesn't appear here.
hussainweb → created an issue.
Shriaas → credited hussainweb → .
volkswagenchick → credited hussainweb → .
I can't find the references I meant to update. Weird... Meanwhile, I'll clean it up a bit.
hussainweb → created an issue.
hussainweb → created an issue.
hussainweb → created an issue.
hussainweb → created an issue.