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

[16.0][ADD] edi_account_edifact_oca #40

Closed

Conversation

@QuocDuong1306 QuocDuong1306 force-pushed the 16.0-add-edi_account_edifact_oca branch 5 times, most recently from c953d00 to f462621 Compare November 10, 2023 02:41
Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

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

Also see my comments on OCA/edi#854

"edi_account_oca",
"edi_edifact_oca",
"account_invoice_edifact",
"edi_storage_oca",
Copy link
Contributor

Choose a reason for hiding this comment

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

This module should not depend on storage. The framework is agnostic for this reason: it does not matter where you send to or receive files from.

_apply_on = ["account.move"]

def on_post_account_move(self, move):
edifact_backend = self.env.ref("edi_edifact_oca.edi_backend_type_edifact")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
edifact_backend = self.env.ref("edi_edifact_oca.edi_backend_type_edifact")
backend_type = self.env.ref("edi_edifact_oca.edi_backend_type_edifact")

def on_post_account_move(self, move):
edifact_backend = self.env.ref("edi_edifact_oca.edi_backend_type_edifact")
if (
not move.partner_id
Copy link
Contributor

Choose a reason for hiding this comment

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

pls move this condition to a specific method. The direction is not needed (see the change that I proposed to the field name in the other PR).

components:
generate:
usage: output.generate.account.move
send:
Copy link
Contributor

Choose a reason for hiding this comment

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

pls remove send

@QuocDuong1306 QuocDuong1306 force-pushed the 16.0-add-edi_account_edifact_oca branch 2 times, most recently from e01bee2 to 2b477d4 Compare November 20, 2023 03:10
@QuocDuong1306
Copy link
Contributor Author

Hi @simahawk , I updated

data = False
exchange_record = self.exchange_record
if exchange_record:
if exchange_record.model == "account.move" and exchange_record.res_id:
Copy link
Contributor

Choose a reason for hiding this comment

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

the model should be selected by _apply_on on the class

def generate(self):
data = False
exchange_record = self.exchange_record
if exchange_record:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think is impossible to not have an exchange record

exchange_record = self.exchange_record
if exchange_record:
if exchange_record.model == "account.move" and exchange_record.res_id:
move = self.env["account.move"].browse(exchange_record.res_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

there's no need to browse. Use self.record.



class EDIExchangeAccountMoveGenerate(Component):
_name = "edi.output.account.move.generate"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
_name = "edi.output.account.move.generate"
_name = "edi.output.edifact.account.move.generate"


* Config `ID number` on the customer and the sender.
* Config `EDI Exchange Type For Invoices` in `Invoicing` tag on the customer.
* Config `Storage` of the EDI backend.
Copy link
Contributor

Choose a reason for hiding this comment

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

storage is not needed. Is not relevant how you send it.

Comment on lines 68 to 73
exchange_record = self.env["edi.exchange.record"].search(
[
("model", "=", "account.move"),
("res_id", "=", self.invoice.id),
]
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
exchange_record = self.env["edi.exchange.record"].search(
[
("model", "=", "account.move"),
("res_id", "=", self.invoice.id),
]
)
exchange_record = self.invoice.exchange_record_ids[0]

Copy link

github-actions bot commented Jun 2, 2024

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.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jun 2, 2024
@QuocDuong1306 QuocDuong1306 force-pushed the 16.0-add-edi_account_edifact_oca branch from 2b477d4 to f7689fc Compare June 3, 2024 01:39
@QuocDuong1306
Copy link
Contributor Author

Thank @simahawk , the PR has been updated as suggested

@github-actions github-actions bot removed the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jun 9, 2024
Copy link

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.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Oct 13, 2024
@github-actions github-actions bot closed this Nov 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants