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.

Sign up for GitHub

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

Jump to bottom

[10.0] partner multi relation tabs migration #498

Merged
merged 6 commits into from
Nov 15, 2019
Merged

[10.0] partner multi relation tabs migration #498

merged 6 commits into from
Nov 15, 2019

Conversation

NL66278
Copy link
Contributor

@NL66278 NL66278 commented Oct 19, 2017

No description provided.

@NL66278 NL66278 force-pushed the 10.0-partner_multi_relation_tabs-migration branch 7 times, most recently from 36f1693 to bdd7f67 Compare October 26, 2017 14:39
@NL66278 NL66278 force-pushed the 10.0-partner_multi_relation_tabs-migration branch from bdd7f67 to 806524c Compare October 28, 2017 17:18
@NL66278
Copy link
Contributor Author

NL66278 commented Oct 29, 2017 •
edited
Loading

This PR depend on/includes #497

(Update 2018-04-13: PR 497 has now been merged)

@NL66278 NL66278 force-pushed the 10.0-partner_multi_relation_tabs-migration branch 2 times, most recently from 3781f25 to b0d7d72 Compare November 2, 2017 00:36
@NL66278 NL66278 changed the title [10.0][WIP] partner multi relation tabs migration [10.0] partner multi relation tabs migration Nov 2, 2017
@NL66278 NL66278 force-pushed the 10.0-partner_multi_relation_tabs-migration branch from b0d7d72 to 358d38c Compare November 2, 2017 14:14
@hbrunn hbrunn added this to the 10.0 milestone Jan 22, 2018
hbrunn
hbrunn requested changes Jan 22, 2018
View reviewed changes
Copy link
Member

@hbrunn hbrunn left a comment

Choose a reason for hiding this comment

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

test coverage needs to be increased

partner_multi_relation/models/res_partner_relation_all.py Outdated Show resolved Hide resolved
@NL66278 NL66278 force-pushed the 10.0-partner_multi_relation_tabs-migration branch from 65ab332 to acfad62 Compare February 9, 2018 18:48
@NL66278 NL66278 force-pushed the 10.0-partner_multi_relation_tabs-migration branch 3 times, most recently from 53206f5 to f2af5aa Compare April 13, 2018 08:50
@NL66278 NL66278 mentioned this pull request Apr 13, 2018
Closed
@NL66278 NL66278 force-pushed the 10.0-partner_multi_relation_tabs-migration branch 2 times, most recently from 34a6ccd to 7ac3a49 Compare April 13, 2018 13:37
@NL66278 NL66278 force-pushed the 10.0-partner_multi_relation_tabs-migration branch from db61276 to 4d2f5e8 Compare April 20, 2018 12:58
@NL66278
Copy link
Contributor Author

NL66278 commented Apr 20, 2018

@hbrunn Tests have been added, so I think your point is satisfied now.

@hbrunn
Copy link
Member

hbrunn commented Apr 23, 2018

thanks!

@NL66278 NL66278 force-pushed the 10.0-partner_multi_relation_tabs-migration branch from 4d2f5e8 to 335585c Compare August 2, 2018 15:04
@NL66278 NL66278 force-pushed the 10.0-partner_multi_relation_tabs-migration branch 2 times, most recently from 70d17c1 to 7643fad Compare November 20, 2018 07:19
@NL66278 NL66278 force-pushed the 10.0-partner_multi_relation_tabs-migration branch 3 times, most recently from 3cdbc3f to 9b6feb4 Compare November 20, 2018 10:13
@NL66278
[IMP] New module partner_multi_relation_tabs.
545ed23
@NL66278
[IMP] partner_multi_relations_tabs. Clean code principles.
530bf80 
- Separate model (db) partially from user interface;
- Use demo data for performance, cleaner tests, and front end testing;
- Make sure tabs dependent on partner_category_id work.
@NL66278 NL66278 force-pushed the 10.0-partner_multi_relation_tabs-migration branch 2 times, most recently from b3af8d7 to 7bda6e2 Compare November 25, 2018 12:56
@NL66278 NL66278 force-pushed the 10.0-partner_multi_relation_tabs-migration branch from 7bda6e2 to 1bb0692 Compare November 25, 2018 19:53
@NL66278
Copy link
Contributor Author

NL66278 commented Nov 26, 2018

@ddufresne Would you be so kind as to have a look at this PR? I also intent to migrate it to 11.0.

@ddufresne
Copy link
Contributor

@NL66278 this is quite a lot of code. Before I go deep into review, I would prefer to see the big picture.

Could you extend your README with screenshots and explanations on how to use your module step by step. Then, it's going to be easier for me to review.

@NL66278 NL66278 force-pushed the 10.0-partner_multi_relation_tabs-migration branch from 2d78b7d to c2a887b Compare November 27, 2018 13:08
@NL66278
Copy link
Contributor Author

NL66278 commented Nov 27, 2018

@ddufresne I added a usage section with screenshots (taking as an example your code for partner_multi_relation in 12.0). However the images will only be rendered properly from the README.rst after the merge, because of the way the url's have been setup. But of course they are part of the code so you should be able to see them there.

@NL66278 NL66278 force-pushed the 10.0-partner_multi_relation_tabs-migration branch from 7b97e4a to 9eb9793 Compare December 22, 2018 09:27
@NL66278 NL66278 requested a review from lfreeke February 21, 2019 09:09
@NL66278
[FIX] partner_multi_relation_tab. Prevent key errors.
74170e6 
There are situations where tabs are added by fields_view_get, but not yet visible for other functions,
presumably in other workers. Handle the errors caused by this. So far only seen in mapped function.
lfreeke
lfreeke approved these changes Nov 15, 2019
View reviewed changes
Copy link
Contributor

@lfreeke lfreeke left a comment

Choose a reason for hiding this comment

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

Functional test 👍

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@hbrunn
Copy link
Member

hbrunn commented Nov 15, 2019

/ocabot merge

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 10.0-ocabot-merge-pr-498-by-hbrunn-bump-no, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Nov 15, 2019
@OCA-git-bot
Merge PR #498 into 10.0
df5c95b 
Signed-off-by hbrunn
@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit 74170e6 into OCA:10.0 Nov 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hbrunn hbrunn hbrunn approved these changes

@lfreeke lfreeke lfreeke approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 needs review ready to merge
Projects
None yet
Milestone
10.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@NL66278 @hbrunn @ddufresne @OCA-git-bot @lfreeke