Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix order values store as string #3222

Merged
merged 4 commits into from
Nov 20, 2017
Merged

Conversation

iakhator
Copy link

@iakhator iakhator commented Nov 2, 2017

Resolve issue #3215

Fix Order values storing as String in the database even when the schema is set as a Number.

Fix

  • In the Cart Schema there is a billing property which has a validation of blackbox set as true is remove.
  • In the PaymentMethod Schema the createAt property with validation of denyUpdate set to true is also remove.

Test.

  • Place an order
  • Open Robomongo or it's alternative
  • Go to billing/invoice and observe the values are stored as Double

screen shot 2017-11-02 at 4 00 05 pm

@brent-hoover
Copy link
Collaborator

It seems like removing the denyOnUpdate is going to mean that every time you update the record the createdAt record will be set which is not what we want. Can you verify this will not happen?

@iakhator
Copy link
Author

iakhator commented Nov 4, 2017

okay, I will check it out

@iakhator
Copy link
Author

iakhator commented Nov 6, 2017

I figured out that denyUpdate is set in the paymentMethod schema which only needs a created date. Because payment is made once after checking out. removing the denyUpdate for the paymentMethod schema does not update.

@brent-hoover brent-hoover self-requested a review November 8, 2017 10:28
brent-hoover
brent-hoover previously approved these changes Nov 9, 2017
Copy link
Collaborator

@brent-hoover brent-hoover left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested. Verified fixed.

@brent-hoover
Copy link
Collaborator

@iakhator You have some tests failing. Lots of them.

Copy link
Collaborator

@brent-hoover brent-hoover left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to address failing tests

@iakhator
Copy link
Author

iakhator commented Nov 9, 2017

Right. I will attend to the failing tests.

@iakhator
Copy link
Author

iakhator commented Nov 9, 2017

Resolved.

@spencern spencern changed the base branch from master to release-1.5.9 November 20, 2017 23:47
@spencern spencern merged commit 565c48b into release-1.5.9 Nov 20, 2017
@spencern spencern deleted the itua-fix-issue-3215 branch November 20, 2017 23:48
@spencern spencern mentioned this pull request Nov 21, 2017
Akarshit pushed a commit that referenced this pull request Jan 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants