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

[ADD] stock_account_move_show_draft_button #1946

Open
wants to merge 1 commit into
base: 16.0
Choose a base branch
from

Conversation

rousseldenis
Copy link

No description provided.

@rousseldenis
Copy link
Author

rousseldenis commented Oct 7, 2024

@rousseldenis
Copy link
Author

After discussion, we need to restrict price changes if there are valuation lines. This will be put in another module

@rousseldenis rousseldenis force-pushed the 16.0-add-account-move-draft-dro branch from d534858 to dfd0296 Compare October 7, 2024 11:21
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.

@rousseldenis Thanks for this PR. I was wondering if the following additional implementation would make sense:

    def _compute_show_reset_to_draft_button(self):
        super()._compute_show_reset_to_draft_button()
        for move in self:
            can_remove_svl = True
            for line in move.invoice_line_ids:
                origin_svl = line.stock_valuation_layer_ids[:1].stock_valuation_layer_id
                if sum(origin_svl.stock_valuation_layer_ids.value) != origin_svl.remaning_value
                    can_remove_svl = False
                    break
            if can_remove_svl:
                move.show_reset_to_draft_button = True

and extend button_draft() to unlink the adjustment SVL and its corresponding account moves, since removal of such records (no subsequent operation has been done to affect the valuation) can be considered to be safe unless I miss something. What do you think?

groups="stock_account_move_show_draft_button.can_bypass_draft_button"
data-hotkey="r"
confirm="You are about to reset the account move to 'draft' state. This accounting entry is related to an inventory valuation. Please note that when you reset to draft, it will not be deleted. That is, if you re-validate the invoice, the valuation will be duplicated.


Recommendation: In developer mode, go to the Inventory/Reports/Valuation menu, group by 'Product', display the product to be adjusted and through the '+' you can generate a manual valuation entry to make the corresponding adjustment. Do you really to do it?"
Copy link
Member

Choose a reason for hiding this comment

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

I think this workaround is OK for AVCO but the situation could get messy for FIFO products. A warning or protection against this operation for FIFO could be nice.

Copy link
Author

Choose a reason for hiding this comment

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

That's why I recommend to use this in parallel with this one if needed: #1947

Copy link
Member

@yostashiro yostashiro Oct 9, 2024

Choose a reason for hiding this comment

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

That's why I recommend to use this in parallel with this one if needed: #1947

This doesn't prevent the problem with FIFO IIUC:

  • 1st receipt of 1pc at EUR10
  • 2nd receipt of 1pc at EUR10 with bill at EUR12 which generates adjustment SVL of EUR2
  • Reset the bill back to draft and repost it (no price change), which generates another adjustment SVL of EUR2
  • Revalue the product for EUR-2, and the remaining value of the SVL of the 1st receipt is now EUR8EUR9 and that of the second EUR11

I suppose we have different cases in mind...?

Copy link
Author

Choose a reason for hiding this comment

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

  • Reset the bill back to draft and repost it (no price change), which generates another adjustment SVL of EUR2

I didn't test this case. I will

"Please ask your administrator."
)
)
self.button_draft()
Copy link
Contributor

Choose a reason for hiding this comment

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

If this development finally stays we should look at including this code and perhaps simplifying it after this change.

Suggested change
self.button_draft()
self.button_draft()
for item in self.sudo().filtered(
lambda x: x.is_inbound
and any(line.stock_valuation_layer_ids for line in x.line_ids)
):
for line in item.line_ids.filtered("stock_valuation_layer_ids"):
svls = line.stock_valuation_layer_ids
value = sum(svls.mapped("value"))
if not float_is_zero(
value, precision_rounding=line.currency_id.rounding
):
svls[0].copy({"value": -value})

Inspired on: OCA/account-invoicing#1813

Copy link
Author

Choose a reason for hiding this comment

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

I don't like having both modules. I will test yours and give feedback today.

Copy link
Contributor

Choose a reason for hiding this comment

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

Of course, I don't want both modules either...
I've put this up because I don't care which one stays, what matters to me is that this is the way forward for a good solution. 😉

@rafaelbn rafaelbn added this to the 16.0 milestone Oct 10, 2024
@gdgellatly
Copy link

I added some thoughts here #1947 (comment) - note this is with a deep understanding of the issues I think these modules are solving, without completely knowing if my understanding of what these modules are solving is correct.

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.

6 participants