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

Tax price fields have wrong naming convention #2918

Closed
2 of 5 tasks
fancydev18 opened this issue May 18, 2019 · 5 comments
Closed
2 of 5 tasks

Tax price fields have wrong naming convention #2918

fancydev18 opened this issue May 18, 2019 · 5 comments
Labels
3: Medium complexity bug Bug reports QA rejected Testers will add this label when something is still wrong refactor Code refactoring
Milestone

Comments

@fancydev18
Copy link

Current behavior

Some object attributes don't respect the naming convention used by all others (underscore, not camelCase)
They are:

priceInclTax 
priceTax 
specialPriceInclTax 
specialPriceTax

Expected behavior

Respect naming convention used by all others, eg price_incl_tax

Steps to reproduce the issue

  1. Open network inspector

  2. Visit
    https://demo.vuestorefront.io/gear/gear-3/joust-duffle-bag-1.html

  3. See response in
    https://demo.storefrontcloud.io/api/catalog/vue_storefront_catalog/product/_search?_source_exclude=%2A.msrp_display_actual_price_type%2Crequired_options%2Cupdated_at%2Ccreated_at%2Cattribute_set_id%2Coptions_container%2Cmsrp_display_actual_price_type%2Chas_options%2Cstock.manage_stock%2Cstock.use_config_min_qty%2Cstock.use_config_notify_stock_qty%2Cstock.stock_id%2Cstock.use_config_backorders%2Cstock.use_config_enable_qty_inc%2Cstock.enable_qty_increments%2Cstock.use_config_manage_stock%2Cstock.use_config_min_sale_qty%2Cstock.notify_stock_qty%2Cstock.use_config_max_sale_qty%2Cstock.use_config_max_sale_qty%2Cstock.qty_increments%2Csmall_image%2Csgn%2C%2A.sgn&from=0&request=%7B%22query%22%3A%7B%22bool%22%3A%7B%22filter%22%3A%7B%22bool%22%3A%7B%22must%22%3A%5B%7B%22terms%22%3A%7B%22category_ids%22%3A%5B4%2C3%5D%7D%7D%2C%7B%22terms%22%3A%7B%22visibility%22%3A%5B2%2C3%2C4%5D%7D%7D%2C%7B%22terms%22%3A%7B%22status%22%3A%5B0%2C1%2C2%5D%7D%7D%5D%7D%7D%7D%7D%7D&size=8&sort=

Repository

Can you handle fixing this bug by yourself?

  • YES
  • NO

Which Release Cycle state this refers to? Info for developer.

Pick one option.

  • This is a bug report for test version on https://test.storefrontcloud.io - In this case Developer should create branch from develop branch and create Pull Request 2. Feature / Improvement back to develop.
  • This is a bug report for current Release Candidate version on https://next.storefrontcloud.io - In this case Developer should create branch from release branch and create Pull Request 3. Stabilisation fix back to release.
  • This is a bug report for current Stable version on https://demo.storefrontcloud.io and should be placed in next stable version hotfix - In this case Developer should create branch from hotfix or master branch and create Pull Request 4. Hotfix back to hotfix.
@fancydev18 fancydev18 added the bug Bug reports label May 18, 2019
@pkarw
Copy link
Collaborator

pkarw commented May 20, 2019

We'll take care of that by the refactoring efforts. We'll apply the following changes:

  • priceInclTax -> price_incl_tax
  • priceTax -> price_tax
  • specialPriceInclTax - special_price_incl_tax
  • specialPriceTax - special_price_tax

We'll keep the older property names as depreciated for some time (would be awesome to add the eslint rule for checking the property names)

@pkarw
Copy link
Collaborator

pkarw commented Jul 5, 2019

Will be closed with #3156

@pkarw pkarw added the QA - Ready for tests This is notification for testers, that improvement is ready to be tested and verified. label Jul 6, 2019
@alinadivante alinadivante added QA rejected Testers will add this label when something is still wrong and removed QA - Ready for tests This is notification for testers, that improvement is ready to be tested and verified. labels Jul 8, 2019
@pkarw
Copy link
Collaborator

pkarw commented Jul 9, 2019

Hi! Thanks for testing this out! @alinadivante this is intended behavior, there is a config.tax.deprecatedPriceFieldsSupport set to true in the vue-storefront-apii for the backward compatibility purposes. When set to false there won't be any of these fields anymore

@alinadivante
Copy link
Collaborator

Thanks @pkarw ! Please include information such as test instructions next time, if there is something what we need to change in config or something like that - let us know! :)
Now I have problem with vue-storefront-api (problem with data migration) I have to wait for @lukeromanowicz time/help ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3: Medium complexity bug Bug reports QA rejected Testers will add this label when something is still wrong refactor Code refactoring
Projects
None yet
Development

No branches or pull requests

3 participants