-
-
Notifications
You must be signed in to change notification settings - Fork 766
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
[14.0][ADD] sale_unreconciled #1283
Conversation
counter_part["debit"] = write_off_vals["credit"] | ||
counter_part["credit"] = write_off_vals["debit"] | ||
counter_part["amount_currency"] = -write_off_vals["amount_currency"] | ||
counter_part["account_id"] = (counterpart_account.id,) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could this throw up a singleton problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it could, I am adding a warning message just in case
everything is reconciled or that the related accounts do not | ||
allow reconciliation""", | ||
) | ||
is_delivered = fields.Boolean(search="_search_is_delivered") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if this feature is adequate in this module, because I would understand that it has more to do with the stock than with the reconciliation of the sales order.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The thing is, this field was part of a previous Odoo version and it was used by this module. But I agree it should be out of this module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The field is removed now.
} | ||
|
||
def action_done(self): | ||
if self.unreconciled: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be refactored to iterate over self, as in the original implementation there is no self.ensure_one ()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point! done :)
sale_unreconciled/models/company.py
Outdated
help="Write-off account to reconcile Unreconciled Sale Orders", | ||
) | ||
|
||
sale_reconcile_journal_id = fields.Many2one("account.journal") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A string should be added in the definition, since the field is visible for the configuration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I don't add a string, it shows as "Sale Reconcile Journal". It think it is correct but I don't mind to put another string if you suggest a different thing.
<filter | ||
name="finance_review" | ||
string="To Review By Finance" | ||
domain="[('unreconciled','=', True), ('invoice_status', '=', 'invoiced'), ('is_delivered', '=', True)]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sounds like a more specific customization
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I think this filter would be the most useful utility in this module, as long as the "Unreconciled " filter will show sales that are unreconciled and they are correct. In any case I am removing this because the field is_delivered is removed as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the filter, it is a custom thing that is not necessary to be in the module
2971dcf
to
51fe015
Compare
9cc3d49
to
85dcf47
Compare
794c775
to
b603ebc
Compare
f11fef3
to
dd5db13
Compare
dd5db13
to
dd9e3e3
Compare
dd9e3e3
to
83a7e4b
Compare
4b0d9aa
to
c3f9c5d
Compare
Could you recreate a runboat? |
[FIX] sale_unreconciled: ensure the accounting settigs are correct [FIX] sale_unreconciled: assign sale order to the write-off created
…concile then assign a matching number [IMP] sale_unreconciled: use the amount resiudal as condition for being unreconciled
… to consider the company of the SO
c3f9c5d
to
397f169
Compare
Just rebased to see if runboat is recreated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Minor nitpicking on grammar.
3a62797
to
d39cc8e
Compare
d39cc8e
to
8ec536a
Compare
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
@alexis-via you might find this pr interesting. |
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
This module adds a new fields "Unreconciled" on Sales Orders, that allows
to find SO's with unreconciled journal items related.
This module allows to reconcile those SO in a single click. In accounting
settings users will be able to set up a specific account for write-off.
cc @ForgeFlow