server-ux icon indicating copy to clipboard operation
server-ux copied to clipboard

[15.0][IMP]base_tier_validation: only post notifications to reciepients

Open bosd opened this issue 2 years ago • 7 comments

Before this PR there where double tier review notifications when used in combination with tier_validation_waiting.

image

One of the messages has no recepients: image

After this PR: Only one notification gets posted. (The one adressed to the requested reviewer) image

bosd avatar Oct 22 '23 21:10 bosd

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

OCA-git-bot avatar Oct 22 '23 21:10 OCA-git-bot

@bosd something is wrong in the tests, can you have a look?

LoisRForgeFlow avatar Oct 23 '23 07:10 LoisRForgeFlow

@bosd something is wrong in the tests, can you have a look?

@LoisRForgeFlow I noticed. I'm working on it right now: https://github.com/OCA/server-ux/pull/733

Yet it is a bit tricky. As I cannot replicate it on my local system. Do you have any ideas?

bosd avatar Oct 23 '23 07:10 bosd

@bosd something is wrong in the tests, can you have a look?

@LoisRForgeFlow I noticed. I'm working on it right now: #733

Yet it is a bit tricky. As I cannot replicate it on my local system. Do you have any ideas?

Not really, I cannot reproduce it locally either...

LoisRForgeFlow avatar Oct 23 '23 08:10 LoisRForgeFlow

Why does the @OCA-git-bot not attach the ready to merge label?

bosd avatar Dec 28 '23 14:12 bosd

Thanks for the review..

  1. When you restart a validation, and request again, you will get no message now in the chatter.

Can you check if the new message after restarting the validation is actually sent out to someone? (Assuming that it is a message without a recepeint) Maybe we can solve this in another way.

2. You are trying to fix a problem in the interaction with base_tier_validation_waiting, however this module is not even available in version 15.0.

It is not yet merged in V15, but there is a PR in https://github.com/OCA/server-ux/pull/638

The tier validation waiting module really adds some usefull features for sequential review. But it also changes the base module drastically which makes it hard to test/maintain. I've proposed to merge it into the base base_tier_validation module. tracked in: https://github.com/OCA/server-ux/pull/787#issuecomment-1871461565 Can you please provide some feedback on that idea there?

bosd avatar Jan 04 '24 07:01 bosd

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[bot] avatar May 05 '24 12:05 github-actions[bot]