Jump to content
  • Checkout
  • Login
  • Get in touch

osCommerce

The e-commerce.

Recommended Posts

  • Replies 213
  • Created
  • Last Reply

@burt,

It seems no more test reports coming in.

Can you indicate, please where to push it.

Thanks

Rainer

Link to comment
Share on other sites

36 minutes ago, burt said:

Assuming I'm looking at the correct Tree on your Github => https://github.com/raiwa/Modular-Checkout

I don't see the needed changes in oscommerce.sql?
I see quite a bit of processing in the main files, which should be moved into module?

Correct branch with oscommerce.sql change is:

https://github.com/raiwa/Responsive-osCommerce/tree/Modular-Checkout-Pages

 

My first exposé was:

On 31/3/2018 at 9:30 AM, raiwa said:

Exposé:

 

-         -  Leave the checks and security part between includes application_top.php and includes template_top.php in the main page files.

-          - Leave the forms in main page file

-          - Move only the content part between includes template_top.php and includes template_bottom.php into content modules

 

Just to ensure everything continues working as it should and assuming all the checks and actions are required in all store.

However, if you wish I can have a look if it is secure and good to move something more into modules.

Link to comment
Share on other sites

7 minutes ago, raiwa said:

However, if you wish I can have a look if it is secure and good to move something more into modules.

:thumbsup:

My idea about Modules is to get all the pages as "thin" as possible, and to have all the modules as "fat" as they need to be..

Link to comment
Share on other sites

11 minutes ago, burt said:

:thumbsup:

My idea about Modules is to get all the pages as "thin" as possible, and to have all the modules as "fat" as they need to be..

I can see it makes sense that obtaining content (eg. get shipping quotes) could be done in the appropriate content module, but the processing of a whole page when confirmed needs to be after actions and before header tags... or doesn't it?

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

Should be possible.  Just needs figuring out what bit of the processing goes in each file...

If you think about it, there's not much point in modularising the display if the processing is still in the file.
If addon maker makes a module that requires some processing...they would still need to process it by amending core.

That's what we need to avoid...is there a way ?

Link to comment
Share on other sites

I had a look into the files and for example in checkout_shipping.php there is no part which I think could be moved.

The security checks and session variable registering is part of all the checkout process and not only needed in the page for a concrete content module.

The part where shipping class is instantiated and all the shipping specific action processes and checks include a lot of redirects which in my opinion should stay before template top.

In general I believe the checkout pages should be treated as an exception because all pages and checks are not isolated like in other pages but part of the whole checkout process and therefore also not linked to one concrete content module.

Even the part concerning the comments are inside action and other checks and should therefore stay before template top:

  if ( isset($_POST['action']) && ($_POST['action'] == 'process') && isset($_POST['formid']) && ($_POST['formid'] == $sessiontoken) ) {
    if (!tep_session_is_registered('comments')) tep_session_register('comments');
    if (tep_not_null($_POST['comments'])) {
      $comments = tep_db_prepare_input($_POST['comments']);
    }

If an add-on needs to interfere in the actions/checks or whatever which are still in the main file, they should use header tag or hooks where needed.

Another point, and please, take it as a constructive critic @burt, all of this should have been discussed at the beginning of the project or at least during the creation process of the modules. Not now when we already asked others for testing one week ago and it's supposed to be ready for push.

My point of view was exposed in the first opening message of this topic.

 

Link to comment
Share on other sites

3 minutes ago, raiwa said:

Another point, and please, take it as a constructive critic @burt, all of this should have been discussed at the beginning of the project or at least during the creation process of the modules. Not now when we already asked others for testing one week ago and it's supposed to be ready for push.

I have nothing to do with this effort, other than passing comment *after* you come up with the code and being a tester. 
My limited look shows more code finesse is needed and I have pointed that out.  Either you get on board with it, or it goes by the wayside (again). 

Put simply, the last time I went to the effort of recoding a load of stuff, I got a dogs abuse by PM and email, I resolved then never to waste my time in that way again.

Link to comment
Share on other sites

@raiwa Rainer I think there's definitely something can be done for the content-gathering - I'll try and get something together this evening.

I think the core edit I make most in shipping is around cheapest()

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

@BrockleyJohn,

yes, this part looks like it makes sense to be moved into the methods module:

// get all available shipping quotes
  $quotes = $shipping_modules->quote();

// if no shipping method has been selected, automatically select the cheapest method.
// if the modules status was changed when none were available, to save on implementing
// a javascript force-selection method, also automatically select the cheapest shipping
// method if more than one module is now enabled
  if ( !tep_session_is_registered('shipping') || ( tep_session_is_registered('shipping') && ($shipping == false) && (tep_count_shipping_modules() > 1) ) ) $shipping = $shipping_modules->cheapest();

 

Link to comment
Share on other sites

in checkout_payment.php it could be this:

// load all enabled payment modules
  require('includes/classes/payment.php');
  $payment_modules = new payment;

 

Link to comment
Share on other sites

It makes sense to have the processing bits in their respective modules so why not pull what we can and see what we're left with.

Dan

Link to comment
Share on other sites

The other thing (that's harder to fit in) is extending the shipping class and so needing a change right at the top

I am definitely not attracted to the idea of putting this section in a module, even ht are too far down, but there are catalog hooks now, what do you think about that?

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

Here is an example;

https://github.com/gburton/Responsive-osCommerce/blob/master/checkout_success.php#L45

  • Form action sets _GET['action'] = update
  • We know always that _GET is in the global scope so can be read anywhere, everywhere...

https://github.com/gburton/Responsive-osCommerce/blob/master/includes/modules/content/checkout_success/cm_cs_product_notifications.php#L42

  • Module reads for _GET['action'] and if it is "update", does some processing

https://github.com/gburton/Responsive-osCommerce/blob/master/checkout_success.php#L33

  • After the module has done that...we go back to the main file, where _GET['action'] is again read, and a redirect is performed

Example:  How Did You Hear About Us

In my 29DoC Project from two years ago, there was a module which allows shopowners to ask how they heard about the site - allows shopowners to add in radio options.  This module did the same thing;

Read for _GET['action'];
Yes:  submit the chosen option to a reporting page
No:  display the options

Example:  Password for Account

Not sure I ever released this, but I ripped out the password stuff from Create Account. 
I then created a checkout_success module for it;

Read for _GET['action'];
Yes:  submit the inputted password to the customers table
No:  display the [required] password input field

The whole idea about Modular is that the module itself takes care of *as much as it possibly can*
I am not saying that modules can do everything, as the base architecture is, let's be polite, "old fashioned".

What I *am* saying...

There is no point introducing modularity if the module then needs a core code change higher up in the main file.

Link to comment
Share on other sites

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 can see that makes it easier to find but it makes it harder to reuse the gathered information in a page because of the sort order... - it means you can only add dependent content lower in the page - or replace the whole thing entirely.

Mind you, for a page like checkout shipping it's not instantly obvious what you'd want to do that didn't involve replacing the methods section.

Actually, having got a little further down the thought process, in the case of checkout_shipping all the processing needs to go in the shipping address module the setup processing needs to go into the shipping address module as that one comes first on the page.

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

@burt based on your suggestions and Johns push, I created a new branch with the checkout shipping page modules including as much as possible the processing codes.

Please have a look:

https://github.com/raiwa/Modular-Checkout/tree/more-modularization_01

the main issue was to move the comment register part into its module and make it stable against sort order changes.

Could this approach be correct/doable in checkout_shipping.php:

  require('includes/languages/' . $language . '/checkout_shipping.php');

// define redirect_to_payment array
  $redirect_to_payment = array();

  $page_content = $oscTemplate->getContent('checkout_shipping');

// redirect to payment page if modules flag true 
  if ( in_array(true, $redirect_to_payment) && !in_array(false, $redirect_to_payment) ) {
    tep_redirect(tep_href_link('checkout_payment.php', '', 'SSL'));
  }

  $breadcrumb->add(NAVBAR_TITLE_1, tep_href_link('checkout_shipping.php', '', 'SSL'));
  $breadcrumb->add(NAVBAR_TITLE_2, tep_href_link('checkout_shipping.php', '', 'SSL'));

and in the shipping methods module replace all:

				  tep_redirect(tep_href_link('checkout_payment.php', '', 'SSL'));

with:

                    $redirect_to_payment[] = true;

 

Link to comment
Share on other sites

on one side it would make things easier on the shipping page.

On the ohter side, the $redirect_to_payment array allows add-on modules to easy interfere in the redirect to payment page.

Link to comment
Share on other sites

checkout payment pushed.

Question: stock check can stay in main page file? It is not directly related to any module.

Link to comment
Share on other sites

@burt, @BrockleyJohn, @Dan Cole, @piernas, @PiLLaO, @Tsimi, @ArtcoInc, @Moxamint, and all others

Finished the modularization of processes.

Full OSC2.3.4.1 BS EDGE Branch (including install sql update):

https://github.com/raiwa/Responsive-osCommerce/tree/modular-checkout

Only modified files:

https://github.com/raiwa/Modular-Checkout/tree/more-modularization_01

 

Tested with core shipping and payment modules. Paypal standard and express in sandbox mode.

Please try and test, if posible also with developer copies of real stores and third party shipping and payment modules.

Please test also with different sort ordes.

 

Known issues- doubts:

1. Redirect array in Checkout shipping:

  • + allows comment module to do its process before redirect to Checkout payment if it’s below shipping method module
  • + allows add-ons to do things before redirect
  • + allows add-ons to interfere in redirect (flag redirect false)
  • – code change

2. Shall the same redirect array be added to checkout_shipping_address and payment_address?:

  • Not needed for core modules
  • Same + for add-ons like above

3.  Checkout payment:

  • Leave payment modules javascript validation in main file or move to tpl_cm_cp_payment_methods.php

 

Thanks for testing

 

Link to comment
Share on other sites

On 4/21/2018 at 8:42 AM, raiwa said:

Known issues- doubts:

1. Redirect array in Checkout shipping:

  • + allows comment module to do its process before redirect to Checkout payment if it’s below shipping method module
  • + allows add-ons to do things before redirect
  • + allows add-ons to interfere in redirect (flag redirect false)
  • – code change

2. Shall the same redirect array be added to checkout_shipping_address and payment_address?:

  • Not needed for core modules
  • Same + for add-ons like above

3.  Checkout payment:

  • Leave payment modules javascript validation in main file or move to tpl_cm_cp_payment_methods.php

 

Rainer are these things you need answers to or looking for feedback on?

Dan

Link to comment
Share on other sites

Archived

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

×
×
  • Create New...