Skip to content

Migrating MassEditing to v10#598

Merged
pedrobaeza merged 5 commits into
OCA:10.0from
JayVora-SerpentCS:mass_editing10
Feb 20, 2017
Merged

Migrating MassEditing to v10#598
pedrobaeza merged 5 commits into
OCA:10.0from
JayVora-SerpentCS:mass_editing10

Conversation

@JayVora-SerpentCS

Copy link
Copy Markdown

No description provided.

@JayVora-SerpentCS JayVora-SerpentCS changed the title Mig to v10 Migrating MassEditing to v10 Nov 10, 2016
@nhomar

nhomar commented Nov 11, 2016

Copy link
Copy Markdown
Member

Thanks @JayVora-SerpentCS

One quick quiestion.

I think the button in the screen that create the action to comply with the >v8.0 usability standard due to the fact that is an "action" button and not a "link" buttton should be in the top of the screen.

I know this is a silly question but I personally think we all will be more happy if we migrate to 10.0 with this little change..

Can You? or do you prefer I make a separate PR?

@jgrandguillaume jgrandguillaume left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @JayVora-SerpentCS

Thanks for this great module !

Tested and works well 👍

Regards,

Joël

@atchuthan

Copy link
Copy Markdown
Member

Found a strange bug when I tried to test the functionality in Runbot

URL: http://3271979-598-6ab5a8.runbot1.odoo-community.org/

Partner Mass editing
image

Mass editing window after selecting 2 partners (ASUSTeK/Test1 & ASUSTeK/Test2)
image

Mass editing window after selecting all partners
image

@pedrobaeza pedrobaeza mentioned this pull request Nov 11, 2016
63 tasks
@pedrobaeza

Copy link
Copy Markdown
Member

@JayVora-SerpentCS, thanks for maintaining this module. Please follow full 10.0 migration guide: https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-10.0#tasks-to-do-in-the-migration and take care about the comments.

You also need to bring the latest changes of the 9.0 branch. For example 65a1579. This is done with https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-10.0#technical-method-to-migrate-a-module-from-90-to-100-branch

@JayVora-SerpentCS

Copy link
Copy Markdown
Author

@pedrobaeza how to bring 65a1579?

@pedrobaeza

Copy link
Copy Markdown
Member

Do:

git remote update
git cherry-pick 65a1579

@njeudy njeudy left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tested and code review

👍

@antoniocanovas antoniocanovas left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM, Tested on runbot.

@pedrobaeza

Copy link
Copy Markdown
Member

The missing commits are still not brought and there's a comment unattended.

@JayVora-SerpentCS

Copy link
Copy Markdown
Author

Agreed, working on them soon.
Thanks.

@ovnicraft

ovnicraft commented Dec 28, 2016

Copy link
Copy Markdown
Member

Hi @JayVora-SerpentCS any updates for this ? is ready to merge ? @pedrobaeza

@JayVora-SerpentCS

Copy link
Copy Markdown
Author

Cherrypicked Pedro's commit. now working on the other 2 comments.

@JayVora-SerpentCS

Copy link
Copy Markdown
Author

@nhomar covered.
screenshot from 2016-12-29 16-13-45
@atchuthan I tried various scenarios, but did not manage to reproduce. Would you please retry?
Thanks.

@ovnicraft

ovnicraft commented Dec 29, 2016

Copy link
Copy Markdown
Member

Tested on runbot and working ok 👍
I cant reproduce @atchuthan bug reported in partners.

@ovnicraft

Copy link
Copy Markdown
Member

There is no report to reproduce the bug mentioned by @atchuthan, IMO this is ready to merge.

@JonathanNEMRY JonathanNEMRY left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hi! Great work! I just made a PR on your branch to add setuptools. Otherwise 👍
JayVora-SerpentCS#1

@JonathanNEMRY JonathanNEMRY left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

@pedrobaeza

Copy link
Copy Markdown
Member

Squashing and merging. Thanks for the work.

@pedrobaeza

Copy link
Copy Markdown
Member

@JayVora-SerpentCS, I'm missing these two important pieces for improving the module that has been done in v8:

Can you please adapt the code for this version?

@pedrobaeza pedrobaeza left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Due to above comment

@JayVora-SerpentCS

Copy link
Copy Markdown
Author

Processed.

@atchuthan atchuthan left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@JayVora-SerpentCS
Sorry for the late follow-up. I couldn't reproduce the error in latest runbot. So, 👍 my vote

@pedrobaeza

Copy link
Copy Markdown
Member

This is not cherry-picking, Jay. A cherry-pick is that you bring the commits as is, not integrating them in another commit with you as the author of the full diff:

seleccion_001

A correct cherry-pick would show in the commit list both commits with the corresponding authors, something like:

seleccion_002

@mourad-ehm

Copy link
Copy Markdown
Contributor

Code review 👍

@LoisRForgeFlow LoisRForgeFlow left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This error pops when trying to add items to a new mass.object record (testing on runbot):
image

Also, please attend runbot warning: WARNING openerp_test odoo.fields: Field mass.object.name: parameter 'select' is no longer supported; use 'index' instead.

hpestano and others added 2 commits February 15, 2017 13:20
As commented by @StefanRijnhart, this is no longer needed after 4442424:

  > This hack in the model's search method allowed for a domain containing a
    stringified list of ids. It has been obsolete since 4442424, and can
    safely be removed (i.e. remove the whole file).

Closes OCA#638.
@JayVora-SerpentCS

Copy link
Copy Markdown
Author

@pedrobaeza You are a nice teacher along with Google! I learned well now the cherry-picking. Earlier commits were squashed and hence it would reflect we ate them, which we actually did not.
I rebased the PR nicely now.
@lreficent Sorry I do not see a reason to face the error. Any detailed traceback possible?
Thanks.

@AaronHForgeFlow AaronHForgeFlow left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When creating a mass editing record on runbot I'm getting an error:
image

@LoisRForgeFlow

Copy link
Copy Markdown
Contributor

@JayVora-SerpentCS I get the error when trying to create a Mass Editing record as usual and trying to add the fields. I have just tested on local right now, same error raises. Aren't you getting the error? Maybe it is my fault but I think it is worth that you test it at least functionally in a moment to test whether it pops for you or not. Really appreciate the effort, thanks!

@JayVora-SerpentCS

Copy link
Copy Markdown
Author

Will check definitely soon and fix if such happens.

@JayVora-SerpentCS

Copy link
Copy Markdown
Author

@aheficent @lreficent True that, made sure no loop hole stays now, edited the domain and fixed.
Thanks.

Comment thread mass_editing/models/mass_object.py Outdated

@api.model
def search(self, args, offset=0, limit=0, order=None, count=False):
print args

@AaronHForgeFlow AaronHForgeFlow Feb 16, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess this is here for debugging and you will remove it. Also, this is causing travis to fail.

The runbot is working now. Thanks.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

For sure, thanks, managed. Had pushed in a hurry :)

@AaronHForgeFlow AaronHForgeFlow left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍 Thanks

@LoisRForgeFlow LoisRForgeFlow left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Works fine now. Thank you!

@pedrobaeza

Copy link
Copy Markdown
Member

@JayVora-SerpentCS, if you want to avoid the forgotten print and pylint/flake8 errors locally, you can install the GIT pre-commit we prepare in MQT: https://github.com/OCA/maintainer-quality-tools/tree/master/git

@JayVora-SerpentCS

Copy link
Copy Markdown
Author

@pedrobaeza I am using pycharm with pep8 and pyflakes extension already.
I will install the precommit too. Thanks.

@pedrobaeza pedrobaeza left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well, finally the history is still no clean (as the other commits don't contain diff), but I don't want to block more time, so I finally squash.

@pedrobaeza
pedrobaeza merged commit 2d47eb4 into OCA:10.0 Feb 20, 2017
mstuttgart pushed a commit to multidadosti-erp/server-tools that referenced this pull request Apr 20, 2017
jnegrete pushed a commit to AlceConsorcio/server-tools that referenced this pull request Nov 11, 2017
ljimenezsidoo pushed a commit to PESOL/server-tools that referenced this pull request Mar 28, 2018
holdenrehg pushed a commit to holdenrehg-forks/server-tools that referenced this pull request Aug 21, 2018
holdenrehg pushed a commit to holdenrehg-forks/server-tools that referenced this pull request Aug 21, 2018
holdenrehg pushed a commit to holdenrehg-forks/server-tools that referenced this pull request Aug 21, 2018
SiesslPhillip pushed a commit to grueneerde/OCA-server-tools that referenced this pull request Nov 20, 2024
Syncing from upstream OCA/server-tools (13.0)
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.