This should fix the issue where a customer, without predefined addresses, is unable to checkout. #219

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
6 participants
@ChrisThompsonTLDR

To replicate the problem, create a customer using the admin area. Do not give that customer an address. Login as that customer and try to checkout.

This commit will assume that if the customer didn't pick "Use existing address" that they are trying to make a new address.

fixes #80

This should fix the issue where a customer, without predefined addres…
…ses, is unable to checkout. To replicate the problem, create a customer using the admin area. Do not give that customer an address. Login as that customer and try to checkout.

This commit will assume that if the customer didn't pick "Use existing address" that they are trying to make a new address.

fixes #80
@stfalcon

This comment has been minimized.

Show comment
Hide comment
@stfalcon

stfalcon Nov 4, 2012

@lokiracer, impossible to see the difference https://github.com/opencart/opencart/pull/219/files

check your IDE end-of-line settings

stfalcon commented Nov 4, 2012

@lokiracer, impossible to see the difference https://github.com/opencart/opencart/pull/219/files

check your IDE end-of-line settings

@ChrisThompsonTLDR

This comment has been minimized.

Show comment
Hide comment
@ChrisThompsonTLDR

ChrisThompsonTLDR Nov 4, 2012

I'm not sure what to check for. My IDE and TortoiseSVN both say only one change was made. Line 169 of the new file replaces line 169-171 of the old file.

What settings do you recommend?

Pic of TortoiseGit and my IDE settings to open files as Unix: https://lh5.googleusercontent.com/-jkmz5FPhrNs/UJbopuiohOI/AAAAAAAAYQU/0SrjJv44qWM/s1280/git.jpg

I'm not sure what to check for. My IDE and TortoiseSVN both say only one change was made. Line 169 of the new file replaces line 169-171 of the old file.

What settings do you recommend?

Pic of TortoiseGit and my IDE settings to open files as Unix: https://lh5.googleusercontent.com/-jkmz5FPhrNs/UJbopuiohOI/AAAAAAAAYQU/0SrjJv44qWM/s1280/git.jpg

@stfalcon

This comment has been minimized.

Show comment
Hide comment
@stfalcon

stfalcon Nov 5, 2012

try change "default file format" for "Windows"

stfalcon commented Nov 5, 2012

try change "default file format" for "Windows"

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 8, 2012

Collaborator

of course you need to make an address. opencart is not designed for some to checkout without one.

Collaborator

ghost commented Nov 8, 2012

of course you need to make an address. opencart is not designed for some to checkout without one.

@ghost ghost closed this Nov 8, 2012

@ChrisThompsonTLDR

This comment has been minimized.

Show comment
Hide comment
@ChrisThompsonTLDR

ChrisThompsonTLDR Nov 8, 2012

And that type of response would be why I dislike helping out projects.

How about contacting me, or looking at the issue. OpenCart makes it impossible to add an address for an existing account because there is no radio button to select "New address."

Take the code or not, OpenCart clearly has a bug for existing users, created by the admin control panel, that don't have addresses.

And that type of response would be why I dislike helping out projects.

How about contacting me, or looking at the issue. OpenCart makes it impossible to add an address for an existing account because there is no radio button to select "New address."

Take the code or not, OpenCart clearly has a bug for existing users, created by the admin control panel, that don't have addresses.

@stfalcon

This comment has been minimized.

Show comment
Hide comment
@stfalcon

stfalcon Nov 8, 2012

@lokiracer don't worry. It's a strange repository.

I also have many ideas to improve OpenCart code and architecture. But I lost desire to improve something after communicating with maintainer.

stfalcon commented Nov 8, 2012

@lokiracer don't worry. It's a strange repository.

I also have many ideas to improve OpenCart code and architecture. But I lost desire to improve something after communicating with maintainer.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 9, 2012

Collaborator

unfortunately i'm very busy. i stopped work on the main script to update
the opencart documentation and in that time i have had to had to stop work
on the that due to the large number of support requests. then had to stop
that to deal with people on Vietnam trying to commit scams on my site.

If I just accepted everyone's suggestion opencart would be a mess. if i
reject some ones changes i have to send the next few weeks listening to
them argue with me.

On Fri, Nov 9, 2012 at 3:09 AM, Stepan Tanasiychuk <notifications@github.com

wrote:

@lokiracer https://github.com/lokiracer don't worry. It's a strange
repository.

I also have many ideas to improve OpenCart code and architecture. But I
lost desire to improve something after communicating with maintainer.


Reply to this email directly or view it on GitHubhttps://github.com/opencart/opencart/pull/219#issuecomment-10200553.

Collaborator

ghost commented Nov 9, 2012

unfortunately i'm very busy. i stopped work on the main script to update
the opencart documentation and in that time i have had to had to stop work
on the that due to the large number of support requests. then had to stop
that to deal with people on Vietnam trying to commit scams on my site.

If I just accepted everyone's suggestion opencart would be a mess. if i
reject some ones changes i have to send the next few weeks listening to
them argue with me.

On Fri, Nov 9, 2012 at 3:09 AM, Stepan Tanasiychuk <notifications@github.com

wrote:

@lokiracer https://github.com/lokiracer don't worry. It's a strange
repository.

I also have many ideas to improve OpenCart code and architecture. But I
lost desire to improve something after communicating with maintainer.


Reply to this email directly or view it on GitHubhttps://github.com/opencart/opencart/pull/219#issuecomment-10200553.

@ChrisThompsonTLDR

This comment has been minimized.

Show comment
Hide comment
@ChrisThompsonTLDR

ChrisThompsonTLDR Nov 9, 2012

Wait. You're complaining to me that you put your codebase on git so people could help you with it?

I didn't make a suggestion, I made a bugfix. A bug that multiple people have documented both here and on the forums. You dismissed it as if I was bothering you with my petty pull request.

I think the words you are looking for are "thank you."

Don't worry about spending more time with this pull request. I won't be bothering you again.

Wait. You're complaining to me that you put your codebase on git so people could help you with it?

I didn't make a suggestion, I made a bugfix. A bug that multiple people have documented both here and on the forums. You dismissed it as if I was bothering you with my petty pull request.

I think the words you are looking for are "thank you."

Don't worry about spending more time with this pull request. I won't be bothering you again.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 9, 2012

Before you get too carried away there you should probably know that your code doesn't fix the bug.

ghost commented Nov 9, 2012

Before you get too carried away there you should probably know that your code doesn't fix the bug.

@DS-Matt

This comment has been minimized.

Show comment
Hide comment
@DS-Matt

DS-Matt Nov 9, 2012

Contributor

Loklracer -

While I tend to think Daniel could exercise a little more diplomacy in his responses to peoples posts/pull requests, I respect his decision to only accept quality code.

Just by adding Opencart to Github doesn't mean Daniel is going to take any and every pull request - not everyone's code is quality and not everyone's pull requests are beneficial to the Opencart Project.

A lot of people submit pull requests that they should spend a little more time on before submitting. People also submit pull requests for non issues in OC, which are pointless to submit. In addition a lot of people post here for help with issues that are NOT BUGS.

If you look back at the pull requests, Daniel did accept several pull requests recently for bug fixes and improvements. I'd encourage you take continue to work on the OC project and try to contribute to the project. Just make sure that you don't take rejections personally.

Best of Luck,

DS-Matt

Contributor

DS-Matt commented Nov 9, 2012

Loklracer -

While I tend to think Daniel could exercise a little more diplomacy in his responses to peoples posts/pull requests, I respect his decision to only accept quality code.

Just by adding Opencart to Github doesn't mean Daniel is going to take any and every pull request - not everyone's code is quality and not everyone's pull requests are beneficial to the Opencart Project.

A lot of people submit pull requests that they should spend a little more time on before submitting. People also submit pull requests for non issues in OC, which are pointless to submit. In addition a lot of people post here for help with issues that are NOT BUGS.

If you look back at the pull requests, Daniel did accept several pull requests recently for bug fixes and improvements. I'd encourage you take continue to work on the OC project and try to contribute to the project. Just make sure that you don't take rejections personally.

Best of Luck,

DS-Matt

@ChrisThompsonTLDR

This comment has been minimized.

Show comment
Hide comment
@ChrisThompsonTLDR

ChrisThompsonTLDR Nov 9, 2012

@opencarthelp I'm not sure what you mean. I tested it and it resolved the issue for me. It seemed fairly simple, the checkout form expects a radio button to be checked indicating a new address or existing address. That radio button is not displayed when an account is created without an address by an admin. The change I made allows the form to submit without that radio button and by default creates a new address.

@DS-Matt I didn't take the rejection personally. I took the tone of his comment personally. I also took the tone of his reply personally.

A lot of people submit pull requests that they should spend a little more time on before submitting. People also submit pull requests for non issues in OC, which are pointless to submit. In addition a lot of people post here for help with issues that are NOT BUGS.

All things that don't apply in this situation. There is clearly a bug, noticed by multiple people, which I submitted a pull request to fix. Instead of considering the bug and my solution, Daniel's response is that I don't understand that OpenCart requires an address. I'm fairly confident I knew that, considering I'm fixing a bug that comes from not being able to checkout because an address can't be added.

@opencarthelp I'm not sure what you mean. I tested it and it resolved the issue for me. It seemed fairly simple, the checkout form expects a radio button to be checked indicating a new address or existing address. That radio button is not displayed when an account is created without an address by an admin. The change I made allows the form to submit without that radio button and by default creates a new address.

@DS-Matt I didn't take the rejection personally. I took the tone of his comment personally. I also took the tone of his reply personally.

A lot of people submit pull requests that they should spend a little more time on before submitting. People also submit pull requests for non issues in OC, which are pointless to submit. In addition a lot of people post here for help with issues that are NOT BUGS.

All things that don't apply in this situation. There is clearly a bug, noticed by multiple people, which I submitted a pull request to fix. Instead of considering the bug and my solution, Daniel's response is that I don't understand that OpenCart requires an address. I'm fairly confident I knew that, considering I'm fixing a bug that comes from not being able to checkout because an address can't be added.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 9, 2012

Line 125. The fact you can't spot it probably goes a long way toward explaining why your merge was rejected.

ghost commented Nov 9, 2012

Line 125. The fact you can't spot it probably goes a long way toward explaining why your merge was rejected.

@ChrisThompsonTLDR

This comment has been minimized.

Show comment
Hide comment
@ChrisThompsonTLDR

ChrisThompsonTLDR Nov 9, 2012

@opencarthelp Sorry, I'm not seeing anything wrong with Line 125 of my commit. Care to explain?

@opencarthelp Sorry, I'm not seeing anything wrong with Line 125 of my commit. Care to explain?

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 9, 2012

Wow. Okay.

So what do you think "Undefined index: payment_address" means?

ghost commented Nov 9, 2012

Wow. Okay.

So what do you think "Undefined index: payment_address" means?

@ChrisThompsonTLDR

This comment has been minimized.

Show comment
Hide comment
@ChrisThompsonTLDR

ChrisThompsonTLDR Nov 9, 2012

I didn't edit line 125, so I'm curious to know why my commit was rejected because of line 125.

Unless you're inferring that the existing customer should call in to that if statement. Because they won't.

Let me lay it out for you again. If an admin makes a customer via the admin area, they aren't required to enter an address. Then the customer logins and tries to checkout and they can't because OpenCart assumes they have an address, which they don't. The customer is shown the address creation form, but without the radio box that fires for line 125 or 171, thus not allowing an address to be added.

I didn't edit line 125, so I'm curious to know why my commit was rejected because of line 125.

Unless you're inferring that the existing customer should call in to that if statement. Because they won't.

Let me lay it out for you again. If an admin makes a customer via the admin area, they aren't required to enter an address. Then the customer logins and tries to checkout and they can't because OpenCart assumes they have an address, which they don't. The customer is shown the address creation form, but without the radio box that fires for line 125 or 171, thus not allowing an address to be added.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 9, 2012

I didn't edit line 125

That's the point! The bug is still there and will stop the checkout from proceeding!

ghost commented Nov 9, 2012

I didn't edit line 125

That's the point! The bug is still there and will stop the checkout from proceeding!

@ChrisThompsonTLDR

This comment has been minimized.

Show comment
Hide comment
@ChrisThompsonTLDR

ChrisThompsonTLDR Nov 9, 2012

125 isn't the cause of this bug. You want me to do a code review on OpenCart for you? There are pages of files/lines that aren't properly written. I'm done replying. Good luck with things.

125 isn't the cause of this bug. You want me to do a code review on OpenCart for you? There are pages of files/lines that aren't properly written. I'm done replying. Good luck with things.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 9, 2012

http://i.imgur.com/kdjVV.png

That's the error I get when following your directions on re-producing.

ghost commented Nov 9, 2012

http://i.imgur.com/kdjVV.png

That's the error I get when following your directions on re-producing.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 9, 2012

Collaborator

even if its a bug i can fix it myself very easily.

i'm not here to jump up and down to fix things for you. I don;t have time to look into this.

its not my fault that people keep replying to a topic that would take me less than 5 minutes to come up with a solution.

Collaborator

ghost commented Nov 9, 2012

even if its a bug i can fix it myself very easily.

i'm not here to jump up and down to fix things for you. I don;t have time to look into this.

its not my fault that people keep replying to a topic that would take me less than 5 minutes to come up with a solution.

@stfalcon

This comment has been minimized.

Show comment
Hide comment
@stfalcon

stfalcon Nov 9, 2012

Gyes, I like OpenCart. But he has a problems with the code and architecture. Do you not understand this?

I don't want edit core files to extend the standart functionality. And I don't want used VQMOD (this is a hack).

I want a flexible and extensible engine. I want to use all the advantages of OOP and PHP 5.3.

PS. Someone working with modern frameworks?

stfalcon commented Nov 9, 2012

Gyes, I like OpenCart. But he has a problems with the code and architecture. Do you not understand this?

I don't want edit core files to extend the standart functionality. And I don't want used VQMOD (this is a hack).

I want a flexible and extensible engine. I want to use all the advantages of OOP and PHP 5.3.

PS. Someone working with modern frameworks?

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 9, 2012

Collaborator

Ok, please list these modern frameworks. If you say zend your in trouble.

Collaborator

ghost commented Nov 9, 2012

Ok, please list these modern frameworks. If you say zend your in trouble.

@stfalcon

This comment has been minimized.

Show comment
Hide comment
@stfalcon

stfalcon Nov 9, 2012

@opencart of PHP frameworks we are using Symfony2

stfalcon commented Nov 9, 2012

@opencart of PHP frameworks we are using Symfony2

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 9, 2012

Collaborator

your wasting my time.

Symfony2 is a bloat framework similar to zend.

you can tell a bloat framework by the number of files required to generate pages. there is about 20 files just for the template system.

opencart template system = 1

Collaborator

ghost commented Nov 9, 2012

your wasting my time.

Symfony2 is a bloat framework similar to zend.

you can tell a bloat framework by the number of files required to generate pages. there is about 20 files just for the template system.

opencart template system = 1

@binaryechoes

This comment has been minimized.

Show comment
Hide comment
@binaryechoes

binaryechoes Nov 9, 2012

Obama can't fix everything.

Obama can't fix everything.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 9, 2012

Collaborator

i'm very busy working on on the documentation system now and have been for the past few weeks but keep having to change the main opencart site to block scammers.

I don't like jumping back an forth between different projects.

people never accept what you say. they always have to get on a high horse and never admit they are wrong. when you run your own open source project that is as big as opencart (now 3rd most popular eCommerce project) then you will find out. people don't take no. they argue with you forever.

I have already done my time messing with frameworks. I have been through zend, cake, Symfony, code inigitor and 100's of other ones posted on sitepoint.

seriously do you think some one who has managed to get a project like this up and running by them selfs have not studied most the frameworks out there.

Why would anyone want such a massive API like Zend or Symfony when its quicker to know the built-in php functions off by heart.

opencart's framework uses a set a simple one file classes. maybe around 20. Symfony's class files goes into the the hundreds. which sounds more optimized to you? which sounds easier to learn.

Collaborator

ghost commented Nov 9, 2012

i'm very busy working on on the documentation system now and have been for the past few weeks but keep having to change the main opencart site to block scammers.

I don't like jumping back an forth between different projects.

people never accept what you say. they always have to get on a high horse and never admit they are wrong. when you run your own open source project that is as big as opencart (now 3rd most popular eCommerce project) then you will find out. people don't take no. they argue with you forever.

I have already done my time messing with frameworks. I have been through zend, cake, Symfony, code inigitor and 100's of other ones posted on sitepoint.

seriously do you think some one who has managed to get a project like this up and running by them selfs have not studied most the frameworks out there.

Why would anyone want such a massive API like Zend or Symfony when its quicker to know the built-in php functions off by heart.

opencart's framework uses a set a simple one file classes. maybe around 20. Symfony's class files goes into the the hundreds. which sounds more optimized to you? which sounds easier to learn.

@willmorgan

This comment has been minimized.

Show comment
Hide comment
@willmorgan

willmorgan Nov 9, 2012

FYI, this popcorn magnet has been posted to reddit:

http://www.reddit.com/r/webdev/comments/12wmr9/textbook_example_of_how_not_to_use_github/

I hope you guys believe in "any publicity is good publicity"...

FYI, this popcorn magnet has been posted to reddit:

http://www.reddit.com/r/webdev/comments/12wmr9/textbook_example_of_how_not_to_use_github/

I hope you guys believe in "any publicity is good publicity"...

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 9, 2012

@justindocanto

@opencart & @opencarthelp, why are you guys even on github?

I have no control over anything here but I seriously wonder the same thing. Daniel can't seem to reject a pull request without it leading to a huge amount of complaining. We're debating frameworks now? Seriously?

DS-Matt is absolutely right. People need to understand the project before jumping into it. I've been submitting bug fixes long before long before Github was used. Sometimes they get used, sometimes they get rewritten, sometimes they get rejected. That's how it works.

Regardless of the debate on everyone's attitude and whether the bug exists, the simple fact is the submitted code doesn't fix it. That should be the end of it right there.

ghost commented Nov 9, 2012

@justindocanto

@opencart & @opencarthelp, why are you guys even on github?

I have no control over anything here but I seriously wonder the same thing. Daniel can't seem to reject a pull request without it leading to a huge amount of complaining. We're debating frameworks now? Seriously?

DS-Matt is absolutely right. People need to understand the project before jumping into it. I've been submitting bug fixes long before long before Github was used. Sometimes they get used, sometimes they get rewritten, sometimes they get rejected. That's how it works.

Regardless of the debate on everyone's attitude and whether the bug exists, the simple fact is the submitted code doesn't fix it. That should be the end of it right there.

@ChrisThompsonTLDR

This comment has been minimized.

Show comment
Hide comment
@ChrisThompsonTLDR

ChrisThompsonTLDR Nov 9, 2012

@opencarthelp It does fix it. It doesn't fix other issues with this file, but it does fix the issue of admin created users without addresses being endlessly redirect during checkout because it's impossible for them to create an address.

I'm sorry I didn't clean up line 125 for you, but it would take a full time salary to fix all the issues with OpenCart. This commit was simply trying to fix a bug, not clean up the codebase.

If you look at /catalog/view/theme/default/template/checkout/payment_address.tpl lines 1-19, you will see that is will be impossible for payment_address to be set by someone that doesn't have an address.

Here are the steps to replicate the bug that this commit fixes.

  1. Login as admin.
  2. Create a user without an address.
  3. Login as user and attempt to checkout.
  4. During step 4 of the checkout process, the checkout will be rejected and the user will be kicked to step 2 without any notification as to why. They will forever be stuck in an endless loop.

This commit assumes that a new address is being created unless existing_address is the value of payment_address. Since you're so stuck on the idea that I didn't fix line 125, simply add an isset() check and call it a day.

@opencarthelp It does fix it. It doesn't fix other issues with this file, but it does fix the issue of admin created users without addresses being endlessly redirect during checkout because it's impossible for them to create an address.

I'm sorry I didn't clean up line 125 for you, but it would take a full time salary to fix all the issues with OpenCart. This commit was simply trying to fix a bug, not clean up the codebase.

If you look at /catalog/view/theme/default/template/checkout/payment_address.tpl lines 1-19, you will see that is will be impossible for payment_address to be set by someone that doesn't have an address.

Here are the steps to replicate the bug that this commit fixes.

  1. Login as admin.
  2. Create a user without an address.
  3. Login as user and attempt to checkout.
  4. During step 4 of the checkout process, the checkout will be rejected and the user will be kicked to step 2 without any notification as to why. They will forever be stuck in an endless loop.

This commit assumes that a new address is being created unless existing_address is the value of payment_address. Since you're so stuck on the idea that I didn't fix line 125, simply add an isset() check and call it a day.

@ghost ghost reopened this Nov 9, 2012

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 9, 2012

Collaborator

@lokiracer banned! just got what you said in the mail.

Collaborator

ghost commented Nov 9, 2012

@lokiracer banned! just got what you said in the mail.

@ghost ghost closed this Nov 9, 2012

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 9, 2012

Collaborator

use magento instead i'm sure they have no bugs at all.

Collaborator

ghost commented Nov 9, 2012

use magento instead i'm sure they have no bugs at all.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 9, 2012

@lokiracer
I'm not denying the issue. Just remove all the addresses from an existing customer in Admin and you can see it.

The problem is all your code does is ignore the error. It's still fundamentally there. It will still fill up your logs. It will still loop through checkout indefinitely if on-page errors are left enabled. This is a bad solution and running off to some other website because you don't like to hear that is goofy.

I think it's far wiser to follow what OpenCart already does and put the solution in the payment_address.tpl template. Just add:

<input type="hidden" name="payment_address" value="new" />

to the fallback form when no addresses are present (~line 21). One line and the issue is really fixed.

ghost commented Nov 9, 2012

@lokiracer
I'm not denying the issue. Just remove all the addresses from an existing customer in Admin and you can see it.

The problem is all your code does is ignore the error. It's still fundamentally there. It will still fill up your logs. It will still loop through checkout indefinitely if on-page errors are left enabled. This is a bad solution and running off to some other website because you don't like to hear that is goofy.

I think it's far wiser to follow what OpenCart already does and put the solution in the payment_address.tpl template. Just add:

<input type="hidden" name="payment_address" value="new" />

to the fallback form when no addresses are present (~line 21). One line and the issue is really fixed.

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Dec 14, 2012

Collaborator

fixed!

Collaborator

ghost commented Dec 14, 2012

fixed!

savage4pro pushed a commit to savage4pro/opencart that referenced this pull request Oct 23, 2017

Merge pull request #219 from nikifalex/image
не нужен полный путь до картинки. Надо просто пусто возвращать

nikolas added a commit to nikolas/github-drama that referenced this pull request Apr 4, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment