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

[12.0] FIX mail_show_follower: ValueError: Expected singleton: res.users #788

Merged
merged 1 commit into from
Feb 28, 2022
Merged

[12.0] FIX mail_show_follower: ValueError: Expected singleton: res.users #788

merged 1 commit into from
Feb 28, 2022

Conversation

eLBati
Copy link
Member

@eLBati eLBati commented Oct 15, 2021

No description provided.

@eLBati
Copy link
Member Author

eLBati commented Oct 20, 2021

@ValentinVinagre what do you think?

@ValentinVinagre
Copy link
Contributor

Hi @eLBati ,
An employee should only have 1 user assigned, in case of having more than one, only the active user should be taken, in this way it would put all the active users or not of that employee.

@eLBati
Copy link
Member Author

eLBati commented Oct 20, 2021

@ValentinVinagre actually this scenario could happen after merging 2 partners: the resulting partner could be linked to 2 active users.
In this case, duplicate users should be handled of course.
But, without this PR, after merging 2 partners and sending an email to the resulting partner would raise an exception and every outgoing email would be blocked.

@ValentinVinagre
Copy link
Contributor

Hummm In this case you need apply the same solution in

social/mail_show_follower/models/mail_mail.py

Line 57 in a523b5d

x.user_ids.groups_id

@eLBati eLBati force-pushed the 12.0-fix-mail_show_follower branch from 9133cc3 to 05e4164 Compare October 21, 2021 06:24
@eLBati
Copy link
Member Author

eLBati commented Oct 21, 2021

Right, thanks, done

@rafaelbn rafaelbn added this to the 12.0 milestone Nov 9, 2021
@eLBati
Copy link
Member Author

eLBati commented Nov 24, 2021

@ValentinVinagre merge?

@ValentinVinagre
Copy link
Contributor

@ValentinVinagre merge?

I am not PSC, I do not have permissions for it.

@eLBati
Copy link
Member Author

eLBati commented Nov 25, 2021

@ValentinVinagre I thought you were maintainer of the module.
Would you like to adopt mail_show_follower?

ValentinVinagre
ValentinVinagre approved these changes Nov 26, 2021
View reviewed changes
Copy link
Contributor

@ValentinVinagre ValentinVinagre left a comment

Choose a reason for hiding this comment

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

👍🏻 LGTM

Shide
Shide approved these changes Feb 17, 2022
View reviewed changes
Copy link
Contributor

@Shide Shide left a comment

Choose a reason for hiding this comment

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

LGTM

@rafaelbn
Copy link
Member

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 12.0-ocabot-merge-pr-788-by-rafaelbn-bump-minor, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Feb 17, 2022
@OCA-git-bot
Merge PR #788 into 12.0
ebebb3d 
Signed-off-by rafaelbn
@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). 🤖

@OCA-git-bot
Copy link
Contributor

@rafaelbn your merge command was aborted due to failed check(s), which you can inspect on this commit of 12.0-ocabot-merge-pr-788-by-rafaelbn-bump-minor.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@rafaelbn
Copy link
Member

/ocabot rebase

@OCA-git-bot
Copy link
Contributor

Congratulations, PR rebased to 12.0.

@rafaelbn
Copy link
Member

/ocabot merge minor

@OCA-git-bot
Copy link
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 12.0-ocabot-merge-pr-788-by-rafaelbn-bump-minor, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Feb 17, 2022
@OCA-git-bot
Merge PR #788 into 12.0
fcb3fe0 
Signed-off-by rafaelbn
@OCA-git-bot
Copy link
Contributor

@rafaelbn your merge command was aborted due to failed check(s), which you can inspect on this commit of 12.0-ocabot-merge-pr-788-by-rafaelbn-bump-minor.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@rafaelbn
Copy link
Member

Hello @eLBati , could you please check #788 (comment)

Thank you! 😄

@MiquelRForgeFlow
Copy link
Contributor

@rafaelbn The error is in extract_msg external library that mail_drop_target uses. Unrelated to this PR. Thus, I think the Pr should be merged manually.

@rafaelbn
Copy link
Member

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 12.0-ocabot-merge-pr-788-by-rafaelbn-bump-patch, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Feb 25, 2022
@OCA-git-bot
Merge PR #788 into 12.0
ec4d313 
Signed-off-by rafaelbn
@OCA-git-bot
Copy link
Contributor

@rafaelbn your merge command was aborted due to failed check(s), which you can inspect on this commit of 12.0-ocabot-merge-pr-788-by-rafaelbn-bump-patch.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@MiquelRForgeFlow
Copy link
Contributor

MiquelRForgeFlow commented Feb 25, 2022 •
edited
Loading

Thus, I think the Pr should be merged manually.

As I said. See #851

@rafaelbn
Copy link
Member

@eLBati , please I'm going to merge manually so I need you change manually the version of the module! Thank! 😄 🙏

@rafaelbn
Copy link
Member

Or @eLBati / @MiquelRForgeFlow the fix is this one (v11) #583

@MiquelRForgeFlow
Copy link
Contributor

@rafaelbn No, that fix was already forward ported in #559 (v12).

@rafaelbn
Copy link
Member

OK, thank you @MiquelRForgeFlow !

@rafaelbn rafaelbn merged commit 49531d6 into OCA:12.0 Feb 28, 2022
rafaelbn added a commit that referenced this pull request Feb 28, 2022
@rafaelbn
[12.0][mail_show_follower] Update version
77fedd0 
As commented in #788 the PR should be merged manually but we should update the version of the module as a minor update

cc @ValentinVinagre @eLBati @MiquelRForgeFlow @HaraldPanten
@rafaelbn rafaelbn mentioned this pull request Feb 28, 2022
Merged
Shide pushed a commit to moduon/social that referenced this pull request Mar 11, 2022
@rafaelbn @Shide
[12.0][mail_show_follower] Update version
a271a65 
As commented in OCA#788 the PR should be merged manually but we should update the version of the module as a minor update

cc @ValentinVinagre @eLBati @MiquelRForgeFlow @HaraldPanten
tafaRU pushed a commit to tafaRU/social that referenced this pull request Mar 18, 2022
@rafaelbn @tafaRU
[12.0][mail_show_follower] Update version
aec5618 
As commented in OCA#788 the PR should be merged manually but we should update the version of the module as a minor update

cc @ValentinVinagre @eLBati @MiquelRForgeFlow @HaraldPanten
MiquelRForgeFlow pushed a commit to ForgeFlow/social that referenced this pull request Jun 22, 2022
@rafaelbn @MiquelRForgeFlow
[12.0][mail_show_follower] Update version
471353c 
As commented in OCA#788 the PR should be merged manually but we should update the version of the module as a minor update

cc @ValentinVinagre @eLBati @MiquelRForgeFlow @HaraldPanten
bosd pushed a commit to bosd/social that referenced this pull request Apr 20, 2023
@rafaelbn @bosd
[12.0][mail_show_follower] Update version
0d56cf0 
As commented in OCA#788 the PR should be merged manually but we should update the version of the module as a minor update

cc @ValentinVinagre @eLBati @MiquelRForgeFlow @HaraldPanten
bosd pushed a commit to bosd/social that referenced this pull request Sep 21, 2023
@rafaelbn @bosd
[12.0][mail_show_follower] Update version
30eb91c 
As commented in OCA#788 the PR should be merged manually but we should update the version of the module as a minor update

cc @ValentinVinagre @eLBati @MiquelRForgeFlow @HaraldPanten
nguyenminhchien pushed a commit to nguyenminhchien/social that referenced this pull request Oct 20, 2023
@rafaelbn @nguyenminhchien
[12.0][mail_show_follower] Update version
c018124 
As commented in OCA#788 the PR should be merged manually but we should update the version of the module as a minor update

cc @ValentinVinagre @eLBati @MiquelRForgeFlow @HaraldPanten
mdurepos pushed a commit to Dur-Pro/social that referenced this pull request Mar 22, 2024
@rafaelbn @mdurepos
[12.0][mail_show_follower] Update version
c160ac9 
As commented in OCA#788 the PR should be merged manually but we should update the version of the module as a minor update

cc @ValentinVinagre @eLBati @MiquelRForgeFlow @HaraldPanten
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Shide Shide Shide approved these changes

@rafaelbn rafaelbn rafaelbn approved these changes

@ValentinVinagre ValentinVinagre ValentinVinagre approved these changes

@MiquelRForgeFlow MiquelRForgeFlow MiquelRForgeFlow approved these changes

@HaraldPanten HaraldPanten HaraldPanten approved these changes

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

Successfully merging this pull request may close these issues.

None yet
7 participants
@eLBati @ValentinVinagre @rafaelbn @OCA-git-bot @MiquelRForgeFlow @Shide @HaraldPanten