Mod Authors Please Note: Smart idea if you're adding configuration entries to addons

[swguy]

Please see USPS, version January 22, 2012 Version F.
http://www.zen-cart.com/index.php?ma...oducts_id=1962

Note in ./includes/modules/shipping/usps.php, lines 125-131

      if (IS_ADMIN_FLAG) {
        $chk_sql = $db->Execute("select * from " . TABLE_CONFIGURATION . " where configuration_key like 'MODULE\_SHIPPING\_USPS\_%' ");
        $chk_keys = $this->keys();
        if (sizeof($chk_keys) != $chk_sql->RecordCount()) {
          $this->title = $this->title . '<span class="alert">' . ' - Missing Keys you should reinstall!' . '</span>';
        }    
      }

If you are adding *new* configuration entries to your shipping (or payment or order total) module, please consider doing a double check like this which verifies that your users have done a REMOVE and then an INSTALL with the new files.

This is the gold standard. Please study it and make sure you understand it.

[RodG]

This is the gold standard. Please study it and make sure you understand it.

I do this slightly differently with the ozpost module.

What the ozpost module does is to store the version number in the database, which is then compared to the version number in the ozpost.php file whenever the check() function is called. If they are different it then calls the remove() and install() functions with no user intervention required.

Oh, it also makes a copy of the current configuration settings b4 calling the remove() function so that it can restore them during the re-install. It then pops up a message advising the user to check their configuration settings for whatever new options have been added.

Needless to say, this takes more coding than "the gold standard" but it also removes the onus on the user to finalise any given upgrade.

I mention my method because I believe the idea of storing and testing version numbers is probably more reliable than your solution (which would fail if a configuration key is simply changed rather than having one or more added or removed).

The automatic backup/remove/install is simply 'icing on the cake'.

Cheers
Rod

[DrByte]

Both approaches have merit.

The one swguy mentioned is most appropriate in the event user-interaction is recommended as a result of the changes. (and in the case of the USPS module that's very much the case, since USPS changed the service options they offer, and so re-selection of preferred choices is important. Backup/restore would be nice, but isn't part of the module at this time.)

The approach RodG mentioned is ideal if no user interaction is needed. If an addon doesn't have internal version number checking anywhere, then a simple check for missing configuration keys or outdated components would be an equally suitable alternative to version checking. The backup/restore of previous settings is of course ideal when there is no need to engage the user to re-select preferences which may be very different as a result of the version change. It's still 'icing on the cake', but if the user still needs to make some new preference changes as a result of the update, there should be some instruction to that effect so that they don't miss that fact.

[RodG]

Both approaches have merit.

With respect, I believe you have missed my point.

The one swguy mentioned is most appropriate in the event user-interaction is recommended as a result of the changes

This is where you seem to have missed the point that this exact same user-interaction is also possible with my solution.

The approach RodG mentioned is ideal if no user interaction is needed.

I *knew* I shouldn't have mentioned the 'icing on the cake', because it has clearly led you into over thinking it.

If an addon doesn't have internal version number checking anywhere, then a simple check for missing configuration keys or outdated components would be an equally suitable alternative to version checking.

This is the significant point of this discussion as far as I'm concerned. Problems can/will only arise if the database is out of sync with the files, so to avoid any such problems this situation needs to be detected somehow.

What happens if/when a mismatch is found is a different ballgame. Either of the proposed solutions can be made to either produce a "Missing Keys you should reinstall!" alert, or, as in the case of ozpost, simply force the re-install.

It is the method of detecting the database/file mismatch that is of interest here.
The 'gold' solution suggested by swguy, is, as I previously stated/implied is flawed in that it will only detect a problem if configuration keys have been added or removed, thus resulting in a count mismatch. The solution that I've implemented actually uses a little less code and is somewhat more reliable because it can/will generate an alert if the configuration_key counts remain constant such as could occur if an old key is removed and a totally different one is added.

It only takes one line of code like:

  $db->Execute("insert into " . TABLE_CONFIGURATION . " (configuration_title, configuration_key, configuration_value, c
onfiguration_description, configuration_group_id, sort_order, date_added) values ('DBversion', 'MODULE_SHIPPING_OZPOST_DB
_VERS', '$this->VERSION', 'Internal use only. Used to ensure databases and files remain syncronised', '6', '90', now())")
;

To initialise the database entry (at the same time as the original module install and/or during the remove/reinstall process).

Then in the check() function another line such as

  if (($this->VERSION) && (MODULE_SHIPPING_OZPOST_DB_VERS) && ($this->VERSION != MODULE_SHIPPING_OZPOST_DB_VERS)) {  
//  generate alert , or do other upgrade stuff // 
}

If swguy's solution is 'the gold' standard, then surely this simpler and more robust method should be considered the 'platinum' standard.

I probably wouldn't have mentioned this other than the fact that I did actually use the configuration key count method myself for a while, until it failed for the exact same reason that it will eventually fail for swguy or others that use this method

I mean no disrespect to swguy or the solution proposed. I saw the flaw/shortcoming as a result of 1st hand experience, and it isn't in my nature to watch others fall into the same traps without saying something.

Cheers
Rod

[swguy]

Rod - no doubt your solution is very nice too. I was recommending Ajeh's approach because it forces the user to consider appropriate values for the config fields, and it's simpler for less experienced mod authors to implement.

[DrByte]

because it has clearly led you into over thinking it.

Actually, you're overthinking my response. I was agreeing that your approach is good.

Enough said.