[IMP] added migration script for filling in the code field from issues - fixes #732

[bodi000]

Hello,

This fixes #732 and simplifies a bit the code, using more odoo mainstream method of getting the defaults.

Please review Thank you B

[bodi000]

@pedrobaeza @ygol Could you please review? Thanks

[pedrobaeza]

Sorry, I'm not using this module.

[bodi000]

@pedrobaeza Sorry to insist, but as it touches migration scripts, could you please just have a look at that part, you are the mig guru :-)

[pedrobaeza]

Migration group is for OpenUpgrade, not for specific modules migration scripts. As I don't know the module logic, I can give any advise.

Even though, I would be glad to analyze the module and the migration scripts if you review 5 random PRs in OCA. This is bi-lateral :wink:

[bodi000]

With great pleasure. I didn't know I could, first of all. I will get back to you once it's done (today I will be out for most of the day ...

On Thu, 22 Oct 2020 at 12:12, Pedro M. Baeza [email protected] wrote:

Migration group is for OpenUpgrade, not for specific modules migration scripts. As I don't know the module logic, I can give any advise.

Even though, I would be glad to analyze the module and the migration scripts if you review 5 random PRs in OCA. This is bi-lateral 😉

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/OCA/project/pull/733#issuecomment-714387676, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZXNDKNWWLN4K323VSDITSMAAPJANCNFSM4S24A5AQ .

-- Bogdan Stanciu

[bodi000]

I don't have to, as it was not there anymore in v12. So someone thought it could go this way. But I am open to discussion, if you like to. I did a quick survey over v11, the two variants are present, with a majority of RO=False.

But again, I see a plus in being able to change the code. don't you?

On Sun, 1 Nov 2020 at 13:49, Stephan Keller [email protected] wrote:

@skeller1 requested changes on this pull request.

In project_task_code/models/project_task.py https://github.com/OCA/project/pull/733#discussion_r515618142:

@@ -8,26 +8,17 @@ class ProjectTask(models.Model): _inherit = 'project.task'

 code = fields.Char(

•    string='Task Number', required=True, default="/", readonly=True)
  

•    string='Task Number',
  

•    required=True,
  

•    default=lambda self: self.env['ir.sequence'].next_by_code(
  

•        'project.task'),
  

•    copy=False)
  

You have to add readonly=True

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/OCA/project/pull/733#pullrequestreview-521221443, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZXNE3SFDWSF5D36Y2QU3SNVKMZANCNFSM4S24A5AQ .

-- Bogdan Stanciu

[skeller1]

@bodi000 Making the code field editable could produce duplicate key errors by user. Have a look at the sales addon: It is not possible to change the order number. The system is generating it for me using ir.sequence. https://github.com/odoo/odoo/blob/d649232e49a8bca55ba8684a605fd34068138cf6/addons/sale/models/sale.py#L143

readonly=True :+1: is the ODOO way, i think

Thank you

[bodi000]

For the sake of completeness, I have to say:

1. you cannot have a duplicate, at most you can get an error. The field is also protected by a SQL unique clause.
2. As I told you, I did an ad hoc survey yesterday, unfortunately odoo looks quite messy on this.

Here are my findings: 20 occurrences of this kind of sequence, i.e. self.env['ir.sequence'].next_by_code('') be it on default or on create with "New" as default (in my view this two step generation is plain inefficient)

Out of these 20 occurrences, 14 have required=True , 16 copy=false (not having this is also wrong in my view) and just 9 readonly=true. Even your example is NOT correct, as sale.order is RO=false in draft state!! readonly=True, states={'draft': [('readonly', False)]} There is even a quite original one, in stock.scrap, with readonly=True, required=True, states={'done': [('readonly', True)]}

So I kindly ask you to accept that you don't have a case. All this being said, if you insist, I will change this RO, no worries. We already spent too much time on a minute issue, in a code base which is far from perfect ;)

b

On Mon, 2 Nov 2020 at 19:39, Stephan Keller [email protected] wrote:

@bodi000 https://github.com/bodi000 Making the code field editable could produce duplicate key errors by user. Have a look at the sales addon: It is not possible to change the order number. The system is generating it for me using ir.sequence.

https://github.com/odoo/odoo/blob/d649232e49a8bca55ba8684a605fd34068138cf6/addons/sale/models/sale.py#L143

readonly=True 👍 is the ODOO way, i think

Thank you

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OCA/project/pull/733#issuecomment-720652961, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZXNAYHCXGH4U2SZMACWTSN34G3ANCNFSM4S24A5AQ .

-- Bogdan Stanciu

[bodi000]

@dreispt Daniel, actually it is not (anymore) as I changed the "logic" too. and I think that the way I proposed is more straightforward.

On Tue, 10 Nov 2020 at 15:11, Daniel Reis [email protected] wrote:

@dreispt commented on this pull request.

In project_task_code/models/project_task.py https://github.com/OCA/project/pull/733#discussion_r520590624:

@@ -8,26 +8,17 @@ class ProjectTask(models.Model): _inherit = 'project.task'

 code = fields.Char(

•    string='Task Number', required=True, default="/", readonly=True)
  

•    string='Task Number',
  

I'm afraid the / default is used by the logic. So you wouldn't touch this part of the code.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/OCA/project/pull/733#pullrequestreview-527239478, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAZXNCDYKDPVKPVXEEQ3MLSPFCZ3ANCNFSM4S24A5AQ .

-- Bogdan Stanciu

[github-actions[bot]]

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.