Hook for Additional Product Language Specific Field

[heatherbell]

Further to this topic - viewtopic.php?f=10&t=1033
I have a hook that inserts an extra field in the Language Specific tab of Product and all seems to work as expected - Yay!
However, I would be very grateful if anyone can point out any fundamental error with my logic in the file.

Code: Select all

<?php

class hook_admin_catalog_productSpecification {
	
  public function listen_productActionSave() {

    if (isset($_GET['pID'])) {
      $products_id = Text::input($_GET['pID']);
    }
    foreach (language::load_all() as $l) {
      $sql_data = Text::prepare($_POST['products_specification'][$l['id']]);

      $GLOBALS['db']->query("UPDATE products_description SET products_specification = '" . $GLOBALS['db']->escape($sql_data) . "' WHERE products_id = '" . (int)$products_id . "' AND language_id = '" . (int)$l['id'] . "'");
    }
  }
 
     public function listen_updateProductAction() {
      $this->listen_productActionSave();
    }

    public function listen_insertProductAction() {
      $this->listen_productActionSave();
    }
    
    public function listen_injectLanguageForm() {
      if (isset($_GET['pID']) && empty($_POST)) {
        $product = product_by_id::administer($_GET['pID']);
        $translations = $product->get('translations');
      }
      foreach (language::load_all() as $l) {
        ?>
        <div class="card-body">
          <div class="form-group row" id="zProductSpecification<?= $l['directory'] ?>">
          <label for="pSpec" class="col-form-label col-sm-3 text-left text-sm-right"><?php echo 'PRODUCT_SPECIFICATIONS'; ?></label>
          <div class="col-sm-9">
            <?= (new Textarea("products_specification[{$l['id']}]", ['id' => 'pSpec', 'cols' => '70', 'rows' => '15']))->require()->set_text($translations[$l['id']]['specification']) ?>
          </div>
          </div>
        </div>
    <?php
        }
      }
    }
    ?>

[burt]

Minimal concern about use of $products_id on L13 if there is no $_GET['pID']

Because L7/8 says if $_GET['pID'] exists set $products_id appropriately.
If no $_GET['pID'] exists...then L13 forces $products_id to zero using INT

So you might end up with a bunch of orphan data.

It's likely on a save that this would never happen, so I would not worry about it at all in terms of "this is for my use only". If it was to be distributed I would probably just wrap the whole lot in L7 something like;

Code: Select all

public function listen_productActionSave() {
    if (isset($_GET['pID'])) {
      $products_id = Text::input($_GET['pID']);
  
      foreach (language::load_all() as $l) {
        $sql_data = Text::prepare($_POST['products_specification'][$l['id']]);

        $GLOBALS['db']->query("UPDATE products_description SET products_specification = '" . $GLOBALS['db']->escape($sql_data) . "' WHERE products_id = '" . (int)$products_id . "' AND language_id = '" . (int)$l['id'] . "'");
      }
    }
  }

[Kofod95]

Either as Burt says or maybe use the global $pID (don't remember what it's actually called, but it's used in some of the supporter's code)?

And just to make sure it's intended: You have the input set as required?

Oh, and I'm unsure that you are pre-populating the field in a way that does indeed populate it? If it works as is, then never mind, but you have ['specification'] there while it's ['products_specification'] elsewhere

//Daniel

[heatherbell]

you have ['specification'] there while it's ['products_specification'] elsewhere

I too thought it should be ['products_specification'] but that broke it - it only worked with ['specification'] but I do not know why

[Kofod95]

but I do not know why

As long as it works

[heatherbell]

If it was to be distributed

Yes, I did not want to publish something here that was a ticking time-bomb!
Hope it's useful to others. Many thanks.

[Kofod95]

In case anyone finds it interesting, here is my guess at why the pre-population has to be ['specification'] and not ['products_specification']:
I think it's because it draws the info through the "language"-class from the "product"-class, and thus ['products_'] is already set via the product-object ($product->specification) - This means it's saves rewriting everything ($product->products_name, $product->products_description etc). Something similar would probably be true for $categories->description and maybe even manufacturers, though it seems like that class has not been fully updated yet. I might be wrong and maybe no one cares, but just to potentially satisfy any curiosity and to force myself to try to understand what I think is right well enough to explain it.

//Daniel

[ecartz]

In general, the Product class assumes that in everything that starts products_ the products_ is unnecessary, so it removes it. For translations, that happens at https://github.com/CE-PhoenixCart/Phoen ... L185..L189

This is mainly so that we don't have to go around writing $product->get('products_blah') all the time, which looks rather redundant.

That already was the way that category_tree worked in 1.0.0.0, although it maps (e.g.) categories_name to name manually. But manufacturer doesn't do any mapping. I doubt that I'll change that in the 1.0.8.* series, as I expect to concentrate on finishing the admin changes and then release.

[heatherbell]

Many thanks to you all for the pointers, explainers, help, support and general encouragement.

Chimpanzee_seated_at_typewriter.jpg

As you can see in image, I'm still busy in the hope that something eventually sinks in, if only by osmosis!
Having some issues with that new hardware though!