πŸ‡³πŸ‡±Netherlands @gidarai

Account created on 2 September 2022, about 2 years ago
#

Recent comments

πŸ‡³πŸ‡±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.

πŸ‡³πŸ‡±Netherlands gidarai

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

πŸ‡³πŸ‡±Netherlands gidarai

Rerolled patch in #62 to work with Drupal 10.3.0

πŸ‡³πŸ‡±Netherlands gidarai

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.

πŸ‡³πŸ‡±Netherlands gidarai

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

πŸ‡³πŸ‡±Netherlands gidarai

Removed unused variables

πŸ‡³πŸ‡±Netherlands gidarai

Removed package and version lines

πŸ‡³πŸ‡±Netherlands gidarai

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

πŸ‡³πŸ‡±Netherlands gidarai

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

πŸ‡³πŸ‡±Netherlands gidarai

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

πŸ‡³πŸ‡±Netherlands gidarai

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.

πŸ‡³πŸ‡±Netherlands gidarai

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

πŸ‡³πŸ‡±Netherlands gidarai

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.

πŸ‡³πŸ‡±Netherlands gidarai

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

πŸ‡³πŸ‡±Netherlands gidarai

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

πŸ‡³πŸ‡±Netherlands gidarai

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.

πŸ‡³πŸ‡±Netherlands gidarai

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

πŸ‡³πŸ‡±Netherlands gidarai

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

πŸ‡³πŸ‡±Netherlands gidarai

Fixed the patch failing to apply (patch corrupted)

πŸ‡³πŸ‡±Netherlands gidarai

@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.

πŸ‡³πŸ‡±Netherlands gidarai

moved $isolation_level line up

πŸ‡³πŸ‡±Netherlands gidarai

Fix custom commands failing

πŸ‡³πŸ‡±Netherlands gidarai

made changes suggested by @daffie

πŸ‡³πŸ‡±Netherlands gidarai

Custom commands failure fix

πŸ‡³πŸ‡±Netherlands gidarai

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.

πŸ‡³πŸ‡±Netherlands gidarai

Fixed patch where custom commands where failing

πŸ‡³πŸ‡±Netherlands gidarai

Regenerated phpstan baseline

πŸ‡³πŸ‡±Netherlands gidarai

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

πŸ‡³πŸ‡±Netherlands gidarai

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

πŸ‡³πŸ‡±Netherlands gidarai

@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();
    }
  }
πŸ‡³πŸ‡±Netherlands gidarai

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

πŸ‡³πŸ‡±Netherlands gidarai

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.

πŸ‡³πŸ‡±Netherlands gidarai

@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.

πŸ‡³πŸ‡±Netherlands gidarai

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.

πŸ‡³πŸ‡±Netherlands gidarai

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.

πŸ‡³πŸ‡±Netherlands gidarai

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

πŸ‡³πŸ‡±Netherlands gidarai

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.

Production build 0.71.5 2024