🇳🇱Netherlands @gidarai

I have created a new patch for this issue, using the previous patch/MR(13) as a basis and incorporating changes from the latest module release.

This is now included & fixed in version 2.0.6. I'm pretty sure we can therefore close this issue.

Rerolled patch in #62 to work with Drupal 10.3.0

Rerolled the patch from #10 and added numerous fixes.

- The 'stock reservation upon adding to cart' feature is now fully optional
- Patch now works with the latest version
- Improved stock form functionality
- Added support for stock enforcement

My colleague (@arantxio.gottmers) and i have been working on this as we need it for our site and thought it would be beneficial to share it here as well.

gidarai → created an issue.

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

Removed unused variables

Removed package and version lines

Added a patch to fix the positioning and size of the corresponding icon

gidarai → created an issue.

Added patch to fix

gidarai → created an issue.

@Tanuja Bohra Yes i was working on the icons for the frontend. Here is the patch including the frontend icon changes.

Changed status back to "needs review" as the code errors aren't related to this patch

changed status

First initial patch

gidarai → created an issue.

We could also maybe get away removing the translate function. As X is not really translatable:|
line 248: 'twitter' => $this->t('X'),

I agree and we could also remove it for the other names (e.g. pinterest, facebook etc.) as well.

Made the "X" variant of the svg a multiline svg, instead of minified

Wrote a patch that changes the twitter icon to the "X" variant, renames the 'Twitter' to 'X' in the form and also changes the api_url to the X variant. Please review.

I have reviewed patch #9. Seems to solve all of the errors/warnings in Drupal 10. Let's move this to RTBC

The patch at #5 works for me, looks good. Im moving this to RTBC so we can hopefully get a release out soon.

It seems that this issue is already fixed in 7.x-1.x-dev, but not yet in 7.x-1.0-rc5, hence why the patch does apply to rc5. I suggest we change the issue version to 7.x-1.0-rc5.

The odd thing is, that it works when applying the patch locally

Fixed patch application. The patch was trying to apply in a/paragraphs/
.inc

Fixed the patch failing to apply (patch corrupted)

@harivansh you seem to have made changes that will break the code as you're trying an array to string conversion. Furthermore the if statement checks if the missing primary key array is not empty to determine wether you are missing primary keys. Please take a better look at the issue next time before making changes and test them accordingly.

moved $isolation_level line up

Fix custom commands failing

made changes suggested by @daffie

Custom commands failure fix

Added different warnings for different isolation levels and when primary keys are missing it shows the corresponding tables that do not have a primary key.

Updated the testIsolationLevelWarningNotDisplaying test to check if the proper warnings/errors are displaying on the status report page.

Fixed patch where custom commands where failing

@Mondrake Of course, go for it!

Regenerated phpstan baseline

Fixed code standards

Added the patch and interdiff files (didn't add it before)

Added a test to check if all tables have a primary key (if not show a warning on the status report page)

Changed PHPStan baseline entry to count 1

@Alexpott i've made the changes you suggested, but didn't really know what to rename the "doGetId" function to. I noticed that the function is used as a sub-function in "getId", but it does indeed do an insert. What do you suggest the new function name to be?

public function getId(): int {
    $try_again = FALSE;
    try {
      // The batch table might not yet exist.
      return $this->doGetId();
    }
    catch (\Exception $e) {
      // If there was an exception, try to create the table.
      if (!$try_again = $this->ensureTableExists()) {
        // If the exception happened for other reason than the missing table,
        // propagate the exception.
        throw $e;
      }
    }
    // Now that the table has been created, try again if necessary.
    if ($try_again) {
      return $this->doGetId();
    }
  }

Added new patch using @mondrake 's suggestion to fix errors on comment #6

Added a new patch which contains an auto increment fix for pgsql from #838992: Change the uid field from integer to serial by leveraging NO_AUTO_VALUE_ON_ZERO on MySQL →

The builds with the latest patch from comment #6 have finished, however it looks like the fallback option will be necessary as the builds resulted in approximately 3k errors.

@catch I agree, it might however be a patch, so i made a new patch without the:

+++ b/core/includes/form.inc
@@ -897,9 +898,29 @@ function batch_process($redirect = NULL, Url $url = NULL, $redirect_callback = N
+    }
+    else {
+      $batch['id'] = rand();
+    }

to test wether it will fail or not.

Changed status to 'postponed' (didn't do it before)

This issue is postponed on https://www.drupal.org/project/drupal/issues/3337513 📌 Fix batch process race conditions by making ‘bid (batch id)’ auto-increment Fixed .

The patch is by @Mondrake from https://www.drupal.org/project/drupal/issues/2665216 📌 Deprecate Drupal\Core\Database\Connection::nextId() and the {sequences} table and schema Fixed and seems like a good patch to start working from.

gidarai → created an issue.

Changed patch to use the lock service to prevent batches from getting the same ID when multiple batch processes try to get a new batch ID by locking the batch process.

Added the requested comment by @catch.

After review on local-machine from daffie i have made some changes and also have fixed the corrupt patch that failed to apply.

gidarai → created an issue.

Created test and rerolled old code changes.
I submitted 2 patches, one with only the test file where the test will fail and another patch with the actual fix where the test succeeds.
Please review.

Removed white space before a closing "}"

Fixed failing test (NextIdTest)