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

[14.0] [MIG] rma module #189

Merged
merged 46 commits into from
Jul 29, 2021
Merged

[14.0] [MIG] rma module #189

merged 46 commits into from
Jul 29, 2021

Conversation

chafique-delli
Copy link
Contributor

No description provided.

@OCA-git-bot
Copy link
Contributor

Hi @ernestotejeda,
some modules you are maintaining are being modified, check this out!

This was referenced Dec 10, 2020
@chafique-delli
Copy link
Contributor Author

@pedrobaeza , ping.

@sebastienbeau
Copy link
Member

Rebased to solve pre-commit issue

@pedrobaeza
Copy link
Member

Please include #193

@chafique-delli
Copy link
Contributor Author

Please include #193
@pedrobaeza , The PR #193, you're telling me is an improvement that only concerns the modules rma_sale and website_rma ?

@pedrobaeza
Copy link
Member

Sorry, you're right. The comment should go in #190

@chafique-delli
Copy link
Contributor Author

Sorry, you're right. The comment should go in #190

Ok, I'll take care of integrating it into the PR #190.

@ryantran-novobi

This comment has been minimized.

Copy link
Member

@ernestotejeda ernestotejeda left a comment

Choose a reason for hiding this comment

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

I think it would be better if you put everything related to the migration in a single commmit:

image

if not self.product_uom or self.product_id.uom_id.id != self.product_uom.id:
self.product_uom = self.product_id.uom_id
return {"domain": {"product_uom": domain_product_uom}}
Copy link
Member

Choose a reason for hiding this comment

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

Why do you think this should be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the Odoo logs, I had this warning:
....onchange method RmaReDeliveryWizard._onchange_product_id returned a domain, this is deprecated

Choose a reason for hiding this comment

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

Solution1:
1.add a field uom_category_id = fields.Many2one(related='product_id.uom_id.category_id')
2. and add on view domain="[('category_id', '=', uom_category_id)]" for product_uom
Solution2:
1.add a field allowed_uom_ids = fields.Many2many('uom.uom', compute="_compute_allowed_uoms")
@api.depends('product_id')
def _compute_allowed_uoms(self):
...
2. and add on view domain="[('id '=', allowed_uom_ids)]" for product_uom

Copy link
Member

@ernestotejeda ernestotejeda left a comment

Choose a reason for hiding this comment

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

Peek 07-04-2021 15-47

Copy link
Member

@ernestotejeda ernestotejeda left a comment

Choose a reason for hiding this comment

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

Peek 07-04-2021 15-48

@chienandalu
Copy link
Member

Please consider forward ports from v12.0 to v13.0 included here https://github.com/OCA/rma/pull/213/commits to avoid functionality regression

Ernesto Tejeda and others added 10 commits June 6, 2021 19:14
[UPD] Update rma.pot

Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: rma-12.0/rma-12.0-rma
Translate-URL: https://translation.odoo-community.org/projects/rma-12-0/rma-12-0-rma/
…flow + Translated using Weblate (Spanish)

[FIX] rma: views permissions

Regular users don't have permissions to rma models, so we should avoid
loading views that lead to permission errors.

TT24986

rma 12.0.1.1.0

Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: rma-12.0/rma-12.0-rma
Translate-URL: https://translation.odoo-community.org/projects/rma-12-0/rma-12-0-rma/

[FIX] rma: portal views access errors

- Portal mail thread needs token config.
- Unpublished products will raise AccessError on RMAs portal views for
portal users due to record rules.
- Ensure active_id when getting actions in rma, since we could come from
a context that pollutes the expected active rma id.

[IMP] rma: teams flow

- If no RMA Team is set, we'll assign a default one to the new RMA.
- A sequence is now used to search for the top team and assign it.
- No default user is assigned when it's not in the context (i.e. portal
rmas).

[UPD] Update rma.pot

rma 12.0.1.2.0

Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: rma-12.0/rma-12.0-rma
Translate-URL: https://translation.odoo-community.org/projects/rma-12-0/rma-12-0-rma/

Translated using Weblate (Spanish)

Currently translated at 96.2% (253 of 263 strings)

Translation: rma-12.0/rma-12.0-rma
Translate-URL: https://translation.odoo-community.org/projects/rma-12-0/rma-12-0-rma/es/
- Fix thrown error when trying to download a picking from the portal.
- Add the hook method to prepare RMA values ​​from the return pick wizard.
- Add the access rule for portal users.
- Show the portal 'Request RMAs' button on the sales page only to users
related to the sales order.

[UPD] Update rma.pot

rma 12.0.1.3.0

Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: rma-12.0/rma-12.0-rma
Translate-URL: https://translation.odoo-community.org/projects/rma-12-0/rma-12-0-rma/
Currently translated at 100.0% (263 of 263 strings)

Translation: rma-12.0/rma-12.0-rma
Translate-URL: https://translation.odoo-community.org/projects/rma-12-0/rma-12-0-rma/es/
- Avoid permission blockings on users who doesn't have RMA permissions.
- Add activities view on RMA list.

[UPD] Update rma.pot

rma 12.0.1.3.1
Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: rma-12.0/rma-12.0-rma
Translate-URL: https://translation.odoo-community.org/projects/rma-12-0/rma-12-0-rma/
- Now it's possible to open several RMAs in a sale order from the portal
- A new comment button has been added to allow the portal user to enter
relevant information like serial numbers o issue description.
- If the requested operation isn't set no RMA will be opened
- The RMA product qty is now a numeric control with limits according to
the qty available to return

[FIX] rma,rma_sale: fix linter errors

[UPD] Update rma.pot

rma 12.0.1.4.0

Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: rma-12.0/rma-12.0-rma
Translate-URL: https://translation.odoo-community.org/projects/rma-12-0/rma-12-0-rma/
Not indicating account.invoice, it takes the default that is the vendor bill view.
We force the suitable one.
…ype use existing lot by default.

[IMP] rma: set rma to received on invoice delete.

Remove 'waiting_refund' rma state.
RMAs go from received to refunded.
When the linked refund is deleted the rma is set to received.

[UPD] Update rma.pot

rma 12.0.1.4.2

Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: rma-12.0/rma-12.0-rma
Translate-URL: https://translation.odoo-community.org/projects/rma-12-0/rma-12-0-rma/

[IMP] rma: incoming picking type use existing lot by default.

rma 12.0.1.4.3
@florian-dacosta
Copy link
Contributor

@pedrobaeza This one seems like a rma_sale fix or am I missing something ?

So it concerns this PR : #190
And I think Chafique will rebase it on top of 13.0, so it should be taken into account when he'll do it

@pedrobaeza
Copy link
Member

Oh, yes sorry, I mixed both.

@chienandalu
Copy link
Member

Can you include 69db691 and 3d63ad8 ?

@florian-dacosta
Copy link
Contributor

Hello @chienandalu I made 2 cherry-picks to take the commit you mention into account.

Could you review so we can try to merge it soon ?
Thanks

@pedrobaeza
Copy link
Member

@ernestotejeda please review to see if something is missing and let's try to merge this ASAP.

Copy link
Member

@ernestotejeda ernestotejeda left a comment

Choose a reason for hiding this comment

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

Error during RMA team creation:

image

Comment on lines 54 to 64
def get_alias_model_name(self, vals):
return vals.get("alias_model", "rma")

def get_alias_values(self):
values = super().get_alias_values()
values["alias_defaults"] = {"team_id": self.id}
return values
Copy link
Member

Choose a reason for hiding this comment

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

It seems these two methods doesn't exist in v14.0

Copy link
Member

@ernestotejeda ernestotejeda left a comment

Choose a reason for hiding this comment

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

image

Copy link
Member

@ernestotejeda ernestotejeda left a comment

Choose a reason for hiding this comment

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

@chafique-delli Apart from the error I mentioned above, everything looks good to me; fix them and I will approve the PR. thanks :).

Copy link
Member

@ernestotejeda ernestotejeda left a comment

Choose a reason for hiding this comment

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

@chafique-delli include this commit 3afd940, please

chafique-delli and others added 4 commits July 28, 2021 17:42
Now we can configure if an automatic notification should be sent when we
receive the goods from an RMA in our warehouse

If we've got `rma_sale` or `website_rma` we can also configure draft
notifications so when the customer places an RMA from the portal the
receive an acknowledge email.

TT29595
Now we can decide which tags are visible for the customer. This way, we
can use them as pseudo-states

TT29594
rma/views/rma_tag_views.xml Outdated Show resolved Hide resolved
rma/views/rma_team_views.xml Outdated Show resolved Hide resolved
@ernestotejeda
Copy link
Member

The previous Suggested Changes fix this error #189 (review)

Co-authored-by: Ernesto Tejeda <ernesto.tejeda87@gmail.com>
@florian-dacosta
Copy link
Contributor

@ernestotejeda Thanks for the suggestion, I applied it. I think all remarks have been attended now.

Copy link
Member

@ernestotejeda ernestotejeda left a comment

Choose a reason for hiding this comment

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

Thank you very much @florian-dacosta @chafique-delli

@pedrobaeza
Copy link
Member

Thanks all for the efforts:

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 14.0-ocabot-merge-pr-189-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit ccaeb38 into OCA:14.0 Jul 29, 2021
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 7c3acf6. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.