Skip to content

Conversation

@AungKoKoLin1997
Copy link
Contributor

@AungKoKoLin1997 AungKoKoLin1997 commented Jan 4, 2023

@AungKoKoLin1997 AungKoKoLin1997 changed the title [ADD] sale_report_qweb_decimal_place [3006][ADD] sale_report_qweb_decimal_place Jan 4, 2023
@AungKoKoLin1997 AungKoKoLin1997 force-pushed the 16.0-add-sale_report_qweb_decimal_place branch from 3e89823 to e4bba51 Compare January 4, 2023 13:47
Copy link
Member

@yostashiro yostashiro left a comment

Choose a reason for hiding this comment

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

@@ -0,0 +1 @@
This module provides the price unit format of sale report depends on the choice of price_decimal_places in currency.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This module provides the price unit format of sale report depends on the choice of price_decimal_places in currency.
This module provides the option to adjust the price unit format (number of decimal places to apply)
of sale reports according to the configuration of price_decimal_places in currency.
This module depends on report_qweb_decimal_place module in OCA/reporting-engine.

@AungKoKoLin1997
Copy link
Contributor Author

@AungKoKoLin1997 Can we put the module in https://github.com/OCA/sale-reporting?

@yostashiro This module is for v 16.0 and we put OCA/reporting-engine#697 is for v 15.0. So, should we create new PR for report_qweb_decimal_place in OCA repo for v16.0 before v15.0 PR is merged? I was planning to create migration PR for report_qweb_decimal_place after this OCA/reporting-engine#697 is merged. That's why I decided to put the modules for v16.0 in fal-custom repo first and later we can put to OCA.

@yostashiro
Copy link
Member

That's fine but why did we target 15.0 in OCA/reporting-engine#697 in the first place? I think there is currently no direct client needs of such module for 15.0.

@AungKoKoLin1997
Copy link
Contributor Author

That's fine but why did we target 15.0 in OCA/reporting-engine#697 in the first place? I think there is currently no direct client needs of such module for 15.0.

I don't know the reason. But in the first place, we have 15.0 branch and I have been assigned to create branch for 16.0 in this task https://www.quartile.co/web#id=3061&model=project.task&view_type=form&menu_id=517 and also create new PR for 16.0 branch in #2 (comment)

Comment on lines 8 to 16
<xpath expr="//td[@name='td_priceunit']/span" position="replace">
<t t-set="currency" t-value="doc.currency_id" />
<t t-set="price_unit" t-value="line.price_unit" />
<t t-call="report_qweb_decimal_place.price_unit_value_format" />
</xpath>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<xpath expr="//td[@name='td_priceunit']/span" position="replace">
<t t-set="currency" t-value="doc.currency_id" />
<t t-set="price_unit" t-value="line.price_unit" />
<t t-call="report_qweb_decimal_place.price_unit_value_format" />
</xpath>
<xpath expr="//span[@t-field='line.price_unit']" position="attributes">
<attribute name="t-att-style">'display: none' if doc.currency_id.apply_price_decimal_place else ''</attribute>
</xpath>
<xpath expr="//span[@t-field='line.price_unit']" position="after">
<t t-if="not doc.currency_id.apply_price_decimal_place">
<t t-set="currency" t-value="doc.currency_id" />
<t t-set="price_unit" t-value="line.price_unit" />
<t t-call="report_qweb_decimal_place.price_unit_value_format" />
</t>
</xpath>

To avoid the overuse of replace.

I was wondering if we should actually change the following template from report_qweb_decimal_place

    <template id="price_unit_value_format">
        <t t-if="currency.apply_price_decimal_place">
            <span
                class="text-nowrap"
                t-esc="price_unit"
                t-options='{"widget": "float", "precision": currency.price_decimal_places}'
            />
        </t>
        <t t-else="">
            <span class="text-nowrap" t-field="line.price_unit" />
        </t>
    </template>

to

    <template id="price_unit_value_format">
        <span
            class="text-nowrap"
            t-esc="price_unit"
            t-options='{"widget": "float", "precision": currency.price_decimal_places}'
        />
    </template>

assuming that the template is called in the above suggested manner. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I think this approach is better.

Copy link
Member

Choose a reason for hiding this comment

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

OK, please go ahead and apply changes to all the relevant places.

@yostashiro
Copy link
Member

@kakurai8 Please go ahead and install the modules in the test database.

@yostashiro yostashiro force-pushed the 16.0-add-sale_report_qweb_decimal_place branch from 7e11154 to a420832 Compare January 28, 2023 15:11
@yostashiro
Copy link
Member

pre-commit fails with this error. Ran copier update to see if there is any material chnages to .pre-commit-config.yaml, however the result was negative.

An unexpected error has occurred: CalledProcessError: command: ('/home/runner/.cache/pre-commit/repop_5djz1u/py_env-python3/bin/python', '-mpip', 'install', '.')
return code: 1
stdout:
    Processing /home/runner/.cache/pre-commit/repop_5djz1u
      Installing build dependencies: started
      Installing build dependencies: finished with status 'done'
      Getting requirements to build wheel: started
      Getting requirements to build wheel: finished with status 'done'
      Preparing metadata (pyproject.toml): started
      Preparing metadata (pyproject.toml): finished with status 'error'
    
stderr:
      error: subprocess-exited-with-error
      
      × Preparing metadata (pyproject.toml) did not run successfully.
      │ exit code: 1
      ╰─> [17 lines of output]
          Traceback (most recent call last):
            File "/home/runner/.cache/pre-commit/repop_5djz1u/py_env-python3/lib/python3.11/site-packages/pip/_vendor/pep[51](https://github.com/qrtl/fal-custom/actions/runs/4032370245/jobs/6932206851#step:7:52)7/in_process/_in_process.py", line 351, in <module>
              main()
            File "/home/runner/.cache/pre-commit/repop_5djz1u/py_env-python3/lib/python3.11/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 333, in main
              json_out['return_val'] = hook(**hook_input['kwargs'])
                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
            File "/home/runner/.cache/pre-commit/repop_5djz1u/py_env-python3/lib/python3.11/site-packages/pip/_vendor/pep517/in_process/_in_process.py", line 1[52](https://github.com/qrtl/fal-custom/actions/runs/4032370245/jobs/6932206851#step:7:53), in prepare_metadata_for_build_wheel
              return hook(metadata_directory, config_settings)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
            File "/tmp/pip-build-env-n3ldi3eb/overlay/lib/python3.11/site-packages/poetry/core/masonry/api.py", line 40, in prepare_metadata_for_build_wheel
              poetry = Factory().create_poetry(Path(".").resolve(), with_groups=False)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
            File "/tmp/pip-build-env-n3ldi3eb/overlay/lib/python3.11/site-packages/poetry/core/factory.py", line [57](https://github.com/qrtl/fal-custom/actions/runs/4032370245/jobs/6932206851#step:7:58), in create_poetry
              raise RuntimeError("The Poetry configuration is invalid:\n" + message)
          RuntimeError: The Poetry configuration is invalid:
            - [extras.pipfile_deprecated_finder.2] 'pip-shims<=0.3.4' does not match '^[a-zA-Z-_.0-9]+$'
          
          [end of output]
      
      note: This error originates from a subprocess, and is likely not a problem with pip.
    error: metadata-generation-failed
    
    × Encountered error while generating package metadata.
    ╰─> See above for output.
    
    note: This is an issue with the package mentioned above, not pip.
    hint: See above for details.
    
Check the log at /home/runner/.cache/pre-commit/pre-commit.log
Error: Process completed with exit code 3.

@yostashiro
Copy link
Member

Ran pre-commit locally and confirmed there is no issue.

@yostashiro yostashiro merged commit 9b899ae into 16.0 Jan 28, 2023
@AungKoKoLin1997 AungKoKoLin1997 deleted the 16.0-add-sale_report_qweb_decimal_place branch March 13, 2023 01:57
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.

4 participants