Jump to content
  • Checkout
  • Login
  • Get in touch

osCommerce

The e-commerce.

Addon Development - Best Practices


Recommended Posts

It would be nice to have a way of dropping extra admin pages into existing boxes without having to update boxes and language.php directly... something like a hook at the bottom of the box def to check in a folder for an additional file.

 

I guess the best place to look would be a subfolder of the language directory because you'd need to load from there anyway for the title

eg. admin\includes\languages\english\boxes\catalog\xsell.php could have the whole entry (not just a constant def) ... or if you don't like corrupting the language files with logic, it needs a non-constant way of passing back the title

Contact me for work on updating existing stores - whether to Phoenix or the new osC when it's released.

Looking for a payment or shipping module? Maybe I've already done it.

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later (DEKO) integration at 2.3.x

Link to comment
Share on other sites

  • Replies 130
  • Created
  • Last Reply

It would be nice to have a way of dropping extra admin pages into existing boxes without having to update boxes and language.php directly... something like a hook at the bottom of the box def to check in a folder for an additional file.

Already exists, more exploration needed on your part :thumbsup:

 

The boxes are shown by an array, so you can drop in a new box to add to the array of any existing box.

 

Take a look at the file: /admin/includes/boxes/tools_security_checks.php for an example.

Also log into an admin of any shop, and check where the link (made in that file) shows...

Link to comment
Share on other sites

Already exists, more exploration needed on your part :thumbsup:

 

The boxes are shown by an array, so you can drop in a new box to add to the array of any existing box.

 

Take a look at the file: /admin/includes/boxes/tools_security_checks.php for an example.

Also log into an admin of any shop, and check where the link (made in that file) shows...

 

thanks @@burt

Contact me for work on updating existing stores - whether to Phoenix or the new osC when it's released.

Looking for a payment or shipping module? Maybe I've already done it.

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later (DEKO) integration at 2.3.x

Link to comment
Share on other sites

I guess I also better go and check if the two template tops now also look for function files so you don't have to edit includes/functions/general.php to make functions generally available.

 

I have to say, there's loads more clever stuff in osC these days.

Contact me for work on updating existing stores - whether to Phoenix or the new osC when it's released.

Looking for a payment or shipping module? Maybe I've already done it.

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later (DEKO) integration at 2.3.x

Link to comment
Share on other sites

I just finished up a modification that adds a new page into the admin area, at the top of the page I did this:

 

 

if (tep_db_num_rows(tep_db_query('SHOW TABLES LIKE "products_slave"')) != 1) {
  tep_db_query("CREATE TABLE products_slave (id int NOT NULL auto_increment, products_id int NOT NULL, links varchar(255) NOT NULL, PRIMARY KEY (id)) CHARACTER SET utf8 COLLATE utf8_unicode_ci;");
}

 

The user uploads the new page, opens it in his browser.  The table is created if it does not already exist.

 

If an addon dev is creating a module (rather than a full admin page), then it is as easy; in the install function of said module the exact same code can be used.

Link to comment
Share on other sites

@@ArtcoInc and I talked about this tables and database stuff in the chat the other day when it suddenly hit me.

There is an addon (commercial) that uses such a feature.

There are radio buttons false/true where you can choose if you want to drop the tables when removing the module or not.

This option can be changed at anytime while the module is installed.

 

So I asked the commercial addon creator if it would be possible to share that part of code and he said yes. (w00t)

So I want to hereby thank @@gadlol for allowing me to share this following code here with you guys. Thank you John! :thumbsup:

 

For testing purpose I implemented it to the reviews module.

Changes are between // BOF TSIMI and // EOF TSIMI

 

http://pastebin.com/RH8Xky0X

 

Have a go and say what you think....

Link to comment
Share on other sites

As @@Tsimi mentioned, we've been working on making add-on's as 'copy - install - configure - and go' as possible. So far, we've:

 

1) avoid editing the database_tables.php file by hard coded the database table names into the code

 

2) avoid editing the filenames.php file by hard coding the filenames directly into the code

 

3) avoid manually running a sql file in phpMyAdmin to create new tables by moving the table creation (and deletion) into the add-on code. This also gives the admin the option to save the tables and data.

(see @@burt 's comments here: http://www.oscommerce.com/forums/topic/407530-addon-development-best-practices/?p=1727142 )

 

My next step is testing the adding of new pages to the admin area, using @@burt 's code here:

http://www.oscommerce.com/forums/topic/407530-addon-development-best-practices/?p=1727637

 

As more hooks are added to the store side, I can see that things are going to get even easier, both for store owners and developers.

 

So, what else does anyone think should be included as a "best practice" in add-on development?

 

Malcolm

 

PS: I want to extend a big 'Thank You' to @@Tsimi , @@burt , @@kymation , and everyone else that has contributed to this little project!

Link to comment
Share on other sites

this is a piece of code ot create a new configuration group at the bottom of the list and you just use @groupid for the group value when creating configuration entries

 

SELECT @sortorder:=max(sort_order) from configuration_group;

INSERT INTO `configuration_group` VALUES ('', 'Group Title', 'This groups description.', @sortorder+1, 1);
SELECT @groupid:=max(configuration_group_id) from configuration_group;

select @groupid;
[

KEEP CALM AND CARRY ON

I do not use the responsive bootstrap version since i coded my responsive version earlier, but i have bought every 28d of code package to support burts effort and keep this forum alive (albeit more like on life support).

So if you are still here ? What are you waiting for ?!

 

Find the most frequent unique errors to fix:

grep "PHP" php_error_log.txt | sed "s/^.* PHP/PHP/g" |grep "line" |sort | uniq -c | sort -r > counterrors.txt

Link to comment
Share on other sites

While it is quite possible to add a new configuration group and various entries in that group (thanks @@bruyndoncx for the code), wouldn't it be preferable to keep the configuration in modules as much as possible? I personally would prefer to have the stock configuration shrink down to only those values that must be the same throughout the store, and leave everything else up to the individual modules. Then the store owner could have, for example, different size images on every page, without having to know which of the many different image sizes in Configuration >> Images applies to each one.

 

Regards

Jim

See my profile for a list of my addons and ways to get support.

Link to comment
Share on other sites

@@Tsimi I've just been looking at something similar. Last week I reworked the cross-sell addon as a product_info content module... latest is here: https://github.com/BrockleyJohn/Responsive-osCommerce/tree/cross_sell_addon

 

Both the admin page and module create the tables if needed. The remove of the content module checks if there's any data, and drops the tables if empty. I would be much happier if it were possible to interrupt processing and ask for a confirmation if the tables contain data (I couldn't immediately see a way of doing it and don't need that for the current project). Using a module setting seems dangerous - it's the sort of thing you'd set to yes when you're implementing and testing the module, then you'd forget about it. If a new release comes along and you have to uninstall and reinstall to pick up new config variables... oops!

 

On the subject of new variables in an upgrade, I put in some hooks to use and extend @@kymation 's validation function concept. Wouldn't it be nice if a new version of the module could recognise you need new config variables and create them for you, keeping all your current ones, instead of requiring an uninstall/reinstall?

Contact me for work on updating existing stores - whether to Phoenix or the new osC when it's released.

Looking for a payment or shipping module? Maybe I've already done it.

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later (DEKO) integration at 2.3.x

Link to comment
Share on other sites

I just finished up a modification that adds a new page into the admin area, at the top of the page I did this:

if (tep_db_num_rows(tep_db_query('SHOW TABLES LIKE "products_slave"')) != 1) {
  tep_db_query("CREATE TABLE products_slave (id int NOT NULL auto_increment, products_id int NOT NULL, links varchar(255) NOT NULL, PRIMARY KEY (id)) CHARACTER SET utf8 COLLATE utf8_unicode_ci;");
}

The user uploads the new page, opens it in his browser.  The table is created if it does not already exist.

 

If an addon dev is creating a module (rather than a full admin page), then it is as easy; in the install function of said module the exact same code can be used.

 

And in another scenario where the module doesn't create a new table but alters an exitisting table by adding a new column, you can check if this column already exists (and avoid an error message) with:

      if (tep_db_num_rows(tep_db_query("SELECT * FROM INFORMATION_SCHEMA.COLUMNS WHERE TABLE_SCHEMA='". DB_DATABASE . "' AND TABLE_NAME='customers' AND COLUMN_NAME LIKE 'customers_guest'")) != 1 ) {
      tep_db_query("alter table customers add column `customers_guest` INT(1) NOT NULL DEFAULT '0' AFTER `customers_newsletter`");
      }
Link to comment
Share on other sites

Wouldn't it be nice if a new version of the module could recognise you need new config variables and create them for you, keeping all your current ones, instead of requiring an uninstall/reinstall?

 

I did that as well. I've added the update method to the standard module class. This allows an update to the module to make additional changes to the database. You can change existing configuration entries (name, description, functions, etc., not just the value), add new configuration entries, add/modify/delete database tables, or anything else you might want to do. The update can be run every time the module values are saved, or only when a checkbox is checked.

 

I used this in the MZMT addon, and a more extensive set in the USPS Codes development code. Both of these add configuration entries after an update without destroying existing entries (unless you ask it to.) USPS Codes also adds/updates a new database table and changes some existing configuration entries to do completely different tasks the second time it is run.

 

This does require modifying admin/modules.php to recognize the new method. It would be nice if this were included in the stock modules.php, or at least a hook provided for it. It's only a few lines of code, but it is immensely powerful.

 

Please take this code and use it, modify it, and release new modules with it. I'd love to see what you do with it. And @@BrockleyJohn: where are those extended validations? In your Cross Sell module? I want to see what you've done.

 

Regards

Jim

See my profile for a list of my addons and ways to get support.

Link to comment
Share on other sites

 

Please take this code and use it, modify it, and release new modules with it. I'd love to see what you do with it. And @@BrockleyJohn: where are those extended validations? In your Cross Sell module? I want to see what you've done.

 

Regards

Jim

 

@@kymation Ah, Jim, so far they're just functions that return a message "this validation doesn't do anything yet, so don't get too excited about the green tick". When they do a bit more, I'll let you know  :thumbsup:  Anyway, they are in the content module. It's my intention to do the version checking & any necessary updates in one of them, so it can be transparent to the shop owner unless it needs decisions making.

Contact me for work on updating existing stores - whether to Phoenix or the new osC when it's released.

Looking for a payment or shipping module? Maybe I've already done it.

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later (DEKO) integration at 2.3.x

Link to comment
Share on other sites

@@kymation I've added some flesh to the cross sell validation functions and pushed them to https://github.com/BrockleyJohn/Responsive-osCommerce/tree/cross_sell_addon - they're in the content module.

 

I'm thinking of a bit of a redesign, now! Checking for all the additional files gives a noticeable delay and that's without the check for edits in existing files which I haven't done yet... this would get annoying every time you focus on the module in admin. So I'm thinking of hanging all the validation off the function that spots you've installed a new version of the module, so it only gets executed once per install/upgrade.

Contact me for work on updating existing stores - whether to Phoenix or the new osC when it's released.

Looking for a payment or shipping module? Maybe I've already done it.

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later (DEKO) integration at 2.3.x

Link to comment
Share on other sites

I wonder what's slow there. It's probably file_exists(), but it would be a good idea to test and find out. I've never noticed any slowing in the modules I used similar functions in, but I might have just missed it. I should probably go research this.

 

Regrds

Jim

See my profile for a list of my addons and ways to get support.

Link to comment
Share on other sites

Here's another guideline, which I always try my best to adhere to;

 

Don't over-engineer

 

Quite often I see 20 lines of code where 1 or 2 can do the same job.  

 

Also some addon makers don't know when to stop coding - it's almost as if they feel that the addon needs to not only do X (which it is being built for) but also Y and Z, and sometimes unrelated A and B and C.

Link to comment
Share on other sites

One of my pet peeves: make your code readable. I've seen some addons here that read like the author was deliberately trying to make the code unreadable. Remember, someone else may end up maintaining what you write someday, so try to make it clear.

 

Spaces and blank lines are useful to organize your code. Don't cram so much into one line that it can't be read on a normal monitor without scrolling. And the most neglected of all: comment your code! It only takes a few seconds to document what you are doing, but it makes reading the code so much easier. Even if it's obvious to you, somebody who is reading it for the first time may not think that your code is all that clear.

 

Regards

Jim

See my profile for a list of my addons and ways to get support.

Link to comment
Share on other sites

@@burt

 

Don't over-engineer

 

Would introducing new classes be an example for that? For example your code for products compare Add-on of 28daysofcode doesn't introduce a class where  I probably would.

 

I am now asking myself ... is a new class really necessary? (for example for wishlist) ... What would be the right coding standard?

Link to comment
Share on other sites

Classes is the way forward, it's coming in future version of osc...

@@wHiTeHaT has done excellent work in this area.

 

As for the particular example "compare products" - there is no need for a class for this and this alone.

When we have a product class in place..that is the time.

Link to comment
Share on other sites

Been reading this post on and off now for a while, and whilst at times I have no idea what you are talking about, I feel that I must give you some form of encouragement for what you are trying to do. As a store owner its good to see that you are all trying to standardize the way things are coded, and the way that addons are created, and maintained. Thats got to be good.

 

I am a great fan of Gary's idea that where possible no core code should be changed when adding an addon. That makes so much sense and makes it so much easier for us store owners to add, try and if necessary remove an addon as it does not really do what we think it would. If all this talk of being able to uninstall the database tables as well without having to read through sql statements to find the added tables, that would be great.

 

The only other thing I could mention is that when the addon is uploaded to the addons area, could there be a way of ensuring that any other upload is firstly working, and is also a full package. May be lock all addons so only the originator of said addon is the only one who can alter it. If someone improves it, they will have to create their own version and support it, or it gets deleted. But thats to come later.

 

Again, well done for what you are trying to do.

REMEMBER BACKUP, BACKUP AND BACKUP

Link to comment
Share on other sites

post-69-0-85042800-1430996265_thumb.jpg

 

The only other thing I could mention is that when the addon is uploaded to the addons area, could there be a way of ensuring that any other upload is firstly working, and is also a full package. May be lock all addons so only the originator of said addon is the only one who can alter it. If someone improves it, they will have to create their own version and support it, or it gets deleted. But thats to come later.

 

This can already be done by the add-on creator.  If you look at any of my addons you will see they are unpolluted (image above).  This is a setting that addon creators can set up themselves.

 

 

 

Link to comment
Share on other sites

about sql, i think something is needed to adres common issues with database changes

- do not include innodb / myisam or other statement

- do not enforce encoding / collation (leave it up to the database default defined)

- defensive programming - do not use if exists drop ...

KEEP CALM AND CARRY ON

I do not use the responsive bootstrap version since i coded my responsive version earlier, but i have bought every 28d of code package to support burts effort and keep this forum alive (albeit more like on life support).

So if you are still here ? What are you waiting for ?!

 

Find the most frequent unique errors to fix:

grep "PHP" php_error_log.txt | sed "s/^.* PHP/PHP/g" |grep "line" |sort | uniq -c | sort -r > counterrors.txt

Link to comment
Share on other sites

attachicon.gifno_public.jpg

 

 

This can already be done by the add-on creator.  If you look at any of my addons you will see they are unpolluted (image above).  This is a setting that addon creators can set up themselves.

 

I know it can, I just wish more people would use it as it would stop many poorly coded add-ons being altered and messed up. I just thought I would mention it.

REMEMBER BACKUP, BACKUP AND BACKUP

Link to comment
Share on other sites

Locking the addon to a single contributor has one major problem: it fails when that contributor stops updating. Example: USPS Rate V4 Intl Rate V2 - v.1.0. The creator of that valuable addon last updated it in February 2012. The constant changes made by the USS made it obsolete shortly after.

 

Others created a new addon and kept it updated, but we still get the occasional user who installs the obsolete (locked) version and then complains in the forum that they installed the "latest" version and it doesn't work. This creates a lot of additional support headaches and wasted effort.

 

The fix for this would be a mechanism that allows somebody else to take over ownership if the original owner does not respond. I don't see this ever happening, so the problem remains.

 

Regards

Jim

See my profile for a list of my addons and ways to get support.

Link to comment
Share on other sites

 

The fix for this would be a mechanism that allows somebody else to take over ownership if the original owner does not respond. I don't see this ever happening, so the problem remains.

 

 

A facility for downloaders to star-rate and review addons would soon highlight the problem, though, and help people know what not to download. But I'm getting off-topic...

Contact me for work on updating existing stores - whether to Phoenix or the new osC when it's released.

Looking for a payment or shipping module? Maybe I've already done it.

Working on generalising bespoke solutions for Quickbooks integration, Easify integration and pay4later (DEKO) integration at 2.3.x

Link to comment
Share on other sites

Archived

This topic is now archived and is closed to further replies.

×
×
  • Create New...