pos-addons icon indicating copy to clipboard operation
pos-addons copied to clipboard

[pos_order_cancel] save pos.cancelled_reason.id in pos.order.line.canceled

Open gaelTorrecillas opened this issue 8 years ago • 9 comments

hello,

it should be a great idea to save pos.cancelled_reason.id in pos.order.line.canceled


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

gaelTorrecillas avatar Sep 22 '17 15:09 gaelTorrecillas

@GabbasovDinar the idea is to add fields.many2many('pos.cancelled_reason') to pos.order.line.canceled model

yelizariev avatar Sep 24 '17 15:09 yelizariev

Hi @yelizariev , actually, you meant that we could have multiple cancelation reason on pos.order.line ? What we're trying to do is to plug BI and reports on cancelation reasons. having each line with a many2one could be more effective maybe?

flotho avatar Sep 25 '17 09:09 flotho

actually, you meant that we could have multiple cancelation reason on pos.order.line ?

I meant multiple cancelation reason on pos.order.line.canceled

What we're trying to do is to plug BI and reports on cancelation reasons. having each line with a many2one could be more effective maybe?

Which many2one field do you mean?

We don't want to restrict module with one reason per line only.

I guess that your concern is that cancled line can be accounted twice in report if you have multiple reasons on that, isn't it? Probably, just add summary on all reasons at the report, e.g.

Line A: 500 - Reason 1
Line B: 500 - Reason 1 and Reason 2
Line C: 1000 - Reason 2

Then in report:


Reason 1: 1000 usd
Reason 2: 1500 usd

Total per Reason: 2500
Total: 2000

yelizariev avatar Sep 26 '17 10:09 yelizariev

Hi @yelizariev,

Thanks for your answers,

I meant multiple cancelation reason on pos.order.line.canceled

You're right, I wrote too fast

I guess that your concern is that canceled line can be accounted twice in report if you have multiple reasons on that, isn't it? Probably, just add summary on all reasons at the report, e.g.

You are totally right regarding the need. I also understand your wish to allow multiple reason on a line. It's of course necessary to prevent any fraud etc...

Maybe we could have an intermediate way which could rely on an intermediate object linked to each line that store the amount and the reason. Thus, the line.cancelled has no multiple instance, the reason text could keep trace of all the reasons and from this line we could see all the reasons with the amount for each of them.

It could turn the model like this :

class PosOrderLineCancelationValue(models.Model):
            _name='pos.order.line.cancelation.value'
            reason_id = fields.Many2one(....)
            line_id = fields.Many2one("pos.order.line.canceled")
            untaxed_amount = ....
            tax_amount = ....
class PosOrderLineCanceled(models.Model):
            _name = "pos.order.line.canceled"
           cancelvalue_id = fields.One2many('pos.order.line.cancelation.value')   

Do you think it could represent a huge evolution?

flotho avatar Sep 26 '17 12:09 flotho

My bad,

with @gaelTorrecillas we made some additionnal tests on your module . As far as we understand, the reason is asked once only when you start modifying the values. Then all the cancelled lines are sent to the server when the transaction is validated. So technically, your line.cancel has only one reason (in text from now): https://github.com/it-projects-llc/pos-addons/blob/10.0/pos_order_cancel/static/src/js/models.js#L44 Have we correctly understand this? If yes, wouldn't it be easier to store a many2one on the line.canceled if a cancellation reason is selected by end user? This way, the end user could continue to input free comment on reason so that it won't be restricted as you want and in the same way, if the end user use a cancellation reason, this particular value will be stored in a many2one.

flotho avatar Sep 26 '17 12:09 flotho

Hi @GabbasovDinar , @yelizariev ,

what is your position on my latest comment https://github.com/it-projects-llc/pos-addons/issues/404#issuecomment-332189522 ?

Thanks for your effective support

flotho avatar Sep 26 '17 14:09 flotho

Hi @GabbasovDinar , @yelizariev ,

during the test for the pr (https://github.com/it-projects-llc/pos-addons/pull/410), with @flotho we saw that in the point of sale, it's possible to select many reasons, we understand the choice of many2many field. It is no longer necessary to keep the amount for each reason.

gaelTorrecillas avatar Sep 27 '17 13:09 gaelTorrecillas

So, we can continue on working implementation as I suggested?

Are there any other concerns we need to pay attention?

yelizariev avatar Sep 28 '17 06:09 yelizariev

hello @yelizariev ,

yes, your seggestion is good, and for the second question, I need your opinion for this PR https://github.com/it-projects-llc/pos-addons/pull/410 and if it's possible to merge

gaelTorrecillas avatar Sep 28 '17 07:09 gaelTorrecillas