@luke.stewart

Note you might also spot this if you upload a CSV with an incorrect number of header rows . Make sure the header does exactly match the template file you download.

I'm unclear about the patch in #5 however this didn't work for me.

What I found was when the error log was not formatted properly this occurred.
We noticed this when transitioning from PHP 7.4 -> 8.1

I have resolved it by addressing https://git.drupalcode.org/project/commerce_smart_importer/-/blob/8.x-1....
and testing to ensure that what we are interating over is an array.

It looks like it's expected that there will be in $fields_log a number of non numeric keys which indicate errors/flags/warnings as a whole as well as numeric keys that relate to specific fields.

I've updated the title to address that this now includes a fix for both the title but also any other field where the field name is the same between the product and the variation.

For example if you have field_image on both product and variation only the variation gets populated. With the attached we can now populate both.

Note. I've not tested this on other functionality of the importer.

This changes the format of the Template used to upload data so any existing imports you need to redownload the template and adjust the data into the relevant columns.

Closing as address this in 3455205

It seems like we can add a third parameter to formatFieldDefinition that defines whether we are calling with a variation or not then we get all fields for product and variations present although weirdly weight is now included after the variation fields for a product.

luke.stewart → created an issue.

luke.stewart → created an issue.

I'm also seeing similar error but on a different route when I proceed to checkout. However a lot of custom code in play. Will update if I track anything down.

Fix for the above at -> https://www.drupal.org/project/commerce_smart_importer/issues/3454069#co... 🐛 PHP 8.x errors + D10 issues Active couldn't figure out how to push to the above repo, and also looks like it's been merged anyway.

Attaching Patchfile for easier consumption.

luke.stewart → created an issue.

The second relates to:

TypeError: count(): Argument #1 ($value) must be of type Countable|array, null given in count()

At -> https://git.drupalcode.org/project/commerce_smart_importer/-/blob/8.x-1....

and
https://git.drupalcode.org/project/commerce_smart_importer/-/blob/8.x-1....

I think the fix for the first is to change to check if the value isset the second a test for empty will suffice? However following this I see a different set of errors for the same imported CSV. Or should this be expected?

Very quick fix for first issue load the logfile into a different variable so we don't have it as a string.

      $logfile = file_get_contents($save . '/log.json');
      if (!empty($logfile)) {
	      $log = json_decode($logfile, TRUE);
      }
    }

I've installed the DEV module on a D10 site. There are a couple of errors that need fixed when running an import.

First:
- I found some weird behaviour with the loading and saving of the log.json file - I think perhas the test to see if it was empty was failing and the file was being loaded and the variable was ending up as a string not an array.

Second:
- There were two errors in the CSV upload form file
($value) must be of type Countable|array, null given in count()
I'm unclear if I've resolved these correctly as when I run and compare the output of field_definitions.json between a working D9 import and Failing D10 one I see the following for example:
Product title doesn't seem to be required on D9 and has no cardinality. On D10 it does with a cardinality of 1. This seems to result in errors regarding the title showing on the post upload screen.

Has anyone else hit something similar and have any advice on resolving?

luke.stewart → created an issue.

Hmm.
Am I missing something here?

When I tested I got the block rendering but not in the right place.

I'm assuming this is because the changes you have made are only to the test to see whether to render the side bar and not to where you are setting the container class:

https://git.drupalcode.org/project/dxpr_theme/-/blob/5.x/templates/page.... and checking it:
https://git.drupalcode.org/project/dxpr_theme/-/blob/5.x/templates/page....
to ensure that the body content region gets the correct bootstrap col classes.

Sorry rereading above I missed a step!

Hopefully the change to this problem/motivation now makes this clear.

I'm wondering if this is actually not possible! or a bug.

We can search some of contrib -> https://git.drupalcode.org/search?group_id=2&scope=blobs&search=sortAggr... but I'm not convinced that any of these examples are trying to do what you are doing.

First from looking at the code I don't think condition is a good example of what the syntax might be.

web/core/lib/Drupal/Core/Entity/Query/QueryBase.php

We see that aggregation was added -> https://www.drupal.org/project/drupal/issues/1854708 →
And it looks like they chose option 3.

So it would suggest we should be able to use the alias but this generates an error.

If we look at what the code does:

It first generates an alias by appending _MAX onto the end of the field name.
Then stores the result indexed by that alias in $this->sortAggregate
And finally adds an aggregate using the passed in field, function and langcode.

So the syntax must align with what aggregate is using.
However when we do this we get an error because the order by ends up on: 'EntityOne'.'EntityTwo'.'field_aggregration_function'
where as the generated column is aliased as 'EntityOneEntityTwofield_aggregrationfunction'

So if you agree with the above I think we should shift this off documentation to be a bug report against the code base?

Thanks for clarifying ( I was just filling in some time by trying to close out some old stale issues.)

I couldn't reproduce your steps above to run the command and see the error messages at which point I figured that unless there was still interest in this there was probably better things for me to be doing with my evening like sleep! :)

Thanks for the additional information.

Just to help reproduce what you are doing above.

You are referencing ebms_packet - are you able to reproduce the workflow you are trying to explain with something from Drupal Core? Or is this workflow dependent on having a custom entity?

For example could we set up a entity reference field on a node and try and do the same thing?

There is a test that demonstrates the usage in:
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/tests/Drupal/...

It's hard to understand exactly what you are trying to attempt here without more information. Do you have the code you used in step 3 try those guesses.

Given elapsed time and the above I'm going to mark Closed (cannot reproduce). Feel free to reopen.

luke.stewart → made their first commit to this issue’s fork.

Given comment 2 by @longwave points out this is limited to some NFS installs with specific config and suggests the solution is best used in contrib, and that there has been no response on this issue since then for 2 years.

I had a quick search of documentation to see if there were any NFS specific pages that we might want to add some information and potentially link to this issue however I didn't spot anything relevant.

Given the above I'm going to mark this as closed (won't fix).

Feel free to reopen if you feel this isn't warranted.

So If I understand the changes I think this is what happened:

Previously attempts per account name were recorded and if they were above the limit set the account was blocked.

However this means that you can test for the existence of an account by triggering the block. So this was removed.

So even if we put a generic message back - that still yields information about the attempt as because it's done within a test to see if the user exists - it's only triggered when a username exists.

So if we want the behaviour back whereby you can trigger a block based on attempting to reset an account separate from the IP block either:

1) We'd need to block the account but not notify the user that got blocked.
2) We'd need to record all attempted usernames in a way that we could block by username even if the username wasn't a username corresponding to a user.

I don't think we could assume that any non existant username is equivalent. Although this would be a nice way to speed up blocking attempts at enumerating usernames it would be possible to use this to identify valid usernames.for example. If you trigger the block and get notified on say 5 usernames that don't exist - and then retry after you are unblocked with 4 that you know don't exist and another username - you would get blocked (and notified) if and only if the fourth didn't exist. Thereby allowing you to test. Although I guess this is complicated by that this behaviour would be influence by any use of the form.

I feel like 1) is worse behaviour. As this would mean that a legitimate user would not be notified they were blocked, leading to confusion as to why they were not getting a password reset email.

For 2 things to consider:

Currently we use the UID in the flood as an identifier - this doesn't work for usernames that don't have a UID because they don't exist.
One option would be to store a list of attempted usernames and a unique id (perhaps a negative integer as this would never be a UID?)
But that seems excessive we could store the UID for existing accounts, and the username for attempts on accounts that don't exist.

What happens if a username that doesn't exist is blocked - is then created. At that point we'd want to remove any block. I think if we split storage between uid for usernames that exist and username for ones that don't we don't have to worry about this.

The only downside I can see with storing non existing usernames and user ids is that it would mean that the storage would be different so would that mean you could use a timing attack to work out if a username exists? Is this something that is worth considering?

So if we wanted to go with option 2 is all that is required to add back a generic error message, and for submissions that valid to meet the test:
if ($account && $account->id() && $account->isActive()) {
call isAllowed with the provided username as the indentifier and if disallowed set the same message.

luke.stewart → created an issue.

Thanks @jurriaanroelofs

Do you remember how to replicate " sidebars regions being rendered even when there were no blocks in the sidebars" it seems like this would impact bootstrap5 theme as well? Would be interested to see what was going on.

luke.stewart → created an issue.

Given the elapsed time. Just reconfirming that this seems stlll to be an issue. I have done the following (on 10.2.4)

Enabled: Content Translation, Language, Interface Translation. Configure related permissions.
Added the first language that showed up in select here: admin/config/regional/language
Enabled Content -> Content types Basic Page and Article to be translatable here: admin/config/regional/content-language

Added a new node of type Basic Page English with something in the body.
Added a translation for language 2

Set Language two to default here: /af/admin/config/regional/language
Deleted Language En here: /af/admin/config/regional/language

Refresh the node.
For the default language get the english version not language 2 as might be expected?
For the previous language 2 page get content not found.

I want to do more digging to replicate the steps in the original post. However It seems like comments 6 and 8 would perhaps agree that a good solution here is to better inform the user of what is actually occurring rather than add additional behaviour.

Can confirm the behaviour in 13 still present in Drupal 10.

Found #1856178: '#group' Form API property only works with

I've just attempted to replicate this on 10.2.4

I created a custom module using drush generate simple:form and installed

In the form I created the following insight the generated buildForm method

    $form['acontainer'] = [
	    '#type' => 'container',
	    '#title' => $this->t('Container'),
    ];

    $form['container']['atextfield'] = [
	    '#type' => 'textarea',
	    '#title' => 'one',
	    '#required' => TRUE,
    ];

When I inspect the elements on the page rendered I see a control with an id 'edit-atextfield' which has a wrapper div with id 'edit-acontainer'

If this doesn't demonstrate the problem please include some clear steps to reproduce.

This should probably be closed or moved to -> https://www.drupal.org/project/aggregator →

Assuming this validator is valid...

https://validator.w3.org/feed/check.cgi?url=drupal.org%2Fnode%2Ffeed

As of now reports that this is a valid feed.
It recommends the following:
description should not contain iframe tag
description should not contain relative URL references:
Missing atom:link with rel="self"

Does this mean that this issue can be closed? (Again?)

I've added a super quick proof of concept in the issue fork - just for the responsive image sub module. In case this highlights what the issue is. there is probably a better way to resolve this!

I've also hit some issues with this module and views.

It seems like perhaps somewhere the css that's being added is being stripped out when the view is rendered.

getBackgroundImageCss is being called and returning the appropriate css to be added.

Further debugging suggests that the issue might be when Big Pipe is enabled the html head is already set and not modified?
If I change elements in viewElements to use the same inline method as this module -> https://www.drupal.org/project/background_image_formatter → we end up with the style tag being rendered inline and working.

Further information to the above.

For a page for which the page height is enough that it's slightly higher than the view port enough to generate scroll bars but not enough that you generate the page fixed behaviour you end up not being able to scroll - as soon as you scroll down the js jumps you back to the top of the page.

My quick fix to resolving this by enforcing a minimum height on the body element such that these pages are always long enough to trigger the proper scrolling behaviour i.e. min-height: 110vh;

I've also manage to generate this. I'm unclear if there was some additional config missing but can confirm that manually updating getClassByPosition such that it defaults to sufix = 12 and only checks $strucutre_id if it's not empty removes the error.

Happy to submit a MR for this if helpful - but you seem to think perhaps this should not be the case? And am wondering if I've done something wrong - missed a step in configuration after install?

I'm using this with Layout Builder on a paragraph if that sheds some light on this?

Thanks for the new composer.json

It still looks like there are some additional requirements in the above that are not marked as requirements by the theme? Or am I not understanding something?

(I'm trying to install based on drupal recommended so as not to hit issues with installing CiviCRM on top of the site, and to match our existing install structure.)

Also trying to get my head around how this works.

I'm assuming it would suffice for example to:

create a new project using drupal recommended
update composer.json to add repos for linkit, webform and components modules
composer require drupal/ckeditor (because Drupal 10 has ckeditor5 which is named differently).
then composer require drupal/civictheme (as per the instructions on the project page)

(I've followed the above steps and just needed to manually install the required modules and seems to be working.)

I am wondering about CKeditor versions - is it possible to just install CKEditor4 but use CKEditor5 from drupal core in text formats. I did spot one warning about a difference in how class styles should be included so perhaps not.

After spotting this demo at DrupalSouth was trying to install to see if it would work on a D7 migration project with limited resource for design. .

In case it's helpful to anyone else I've done the following to get it installed on D10.0:
The projects for which Civictheme requires a version without D10 support all have some form of D10 release.
linkit -> either 6.1.0-rc1 or 6.0.0-rc1
components -> 3.0.0-beta3
webform -> 6.2.0-beta6

So we can use aliases to get these installed and then trick composer into thinking they are a supported version that Civictheme requires and cross our fingers and hope that the version increment doesn't break anything!

I guess another way to do this would be to fork civictheme and then update the composer requirements to allow the above versions.

For each project we first use composer show --all drupal/projectname to see what versions are on offer then pick the latest version that will support D10 and alias to the latest version that will match requirements in https://git.drupalcode.org/project/civictheme/-/blob/1.x/composer.json

composer require 'drupal/webform:6.2.0-beta6 as 6.1'
composer require 'drupal/components:3.0.0-beta3 as 2.4.0'

Note - linkit will work for drupal 10.0 just not 10.1

The only other fish hook is this theme depends on ckeditor4 - which doesn't exist in drupal core in D10 - however we can install the contrib module.

Looking at this:

This issue dates back to D7 and is flagged with needs backport - but in this stage of D7 lifecycle I think this can be removed as this bug relates to new installs - and one would hope new installs of D7 are extremely rare at this point!

1093420 is an attempt to address the impacts of this bug when testing based on install profiles this seems to indicate this is still a problem - but only in D7 - there is no evidence for D8+

Reading back over the issue

I'm flagging this as needs manual testing. As I don't see any evidence suggesting that this is an issue in D9/10/11. If it is we definitely should address. But the implementation has changed somewhat and based on the comments of the method in D11 branch I'm not convinced this is still a problem.

Summarised patch and added to the proposed solution. Removed the needs issue summary update.

My suspicion would be that we need to consider Day Light savings here.

Additionally the REQUEST_TIME global constant has also been deprecated
https://www.drupal.org/node/2785211 →

Currently the patch assumes each day is 24 hours long. Perhaps the use of datetime.diff would prevent shifts occurring due to day light savings.

The error generated is:
Symfony\Component\Routing\Exception\RouteNotFoundException: Route "menu.api.search" does not exist. in Drupal\Core\Routing\RouteProvider->getRouteByName() (line 206 of /web/core/lib/Drupal/Core/Routing/RouteProvider.php).

Would it be a bad solution to just remove the second half of the example?

Updated the version target.

I've updated the issue summary based on https://www.drupal.org/project/drupal/issues/2371863#comment-14607943 🐛 SiteInformationForm validates access to paths that aren't changed, potentially making the form unsubmittable Needs work

Given the potential for data loss perhaps this should be updated from Minor?

https://github.com/eileenmcnaughton/civicrm_entity/pull/425

I've applied the patch against the 4.x version, and tested - originally written for d9 with 3.x version seems to work fine on D10.

I've left in some logging - that perhaps should be removed.

luke.stewart → created an issue.