[12.0][MIG] date_range #25

astirpe · 2018-10-01T14:44:51Z

No description provided.

* [ADD] Basic structure for the new date range module * [IMP] Add a basic description into the README * [IMP] Basic implementation * [IMP] First working implementation * [IMP] Improve datamodel * [ADD] Add basic tests for date.range * [PEP8] * [PYLINT] * [DEL] Remove unused code * [IMP] Remove unsused dependencies into the JS * [IMP] Better operator label for date range * [DEL] Remove unused file * [IMP] Better user experience by showing the select input only once empty * [FIX]Try to fix tests that fails only on travis by adding an explicit cast on the daterange methods parameters * [FIX]Try to fix tests that fails only on travis by adding an explicit cast on the daterange methods parameters * [FIX]Try to fix tests that fails only on travis by using postgresql 9.4 * [FIX]Try with postgresql 9.2 since the daterange method has appeared in 9.2 * [IMP] Add a limitation into the module description to warm about the minimal version of postgresql to use * [IMP]Add multi-company rules * [IMP]Remove unused files * [FIX] Add missing brackets into JS * [FIX] Overlap detection when company_id is False * [IMP] Add default order for date.range * [IMP] Add date range generator * [FIX] OE compatibility * [FIX] Travis * [IMP] Code cleanup and improves test coverage * [FIX] Add missing dependency on 'web' * [PYLINT] remove unused import * [FIX] Add missing copyright * [FIX] Limits are included into the range * [IMP][date_range] Security * [IMP] Improve module description * [IMP] Spelling

* Improve 'name' for generator wizard ir.rule should be active by default * Don't auto-add '-' after prefix when generating date ranges via wizard * code fine tuning suggested by Sylvain Garancher

Currently translated at 89.1% (41 of 46 strings) Translation: server-ux-11.0/server-ux-11.0-date_range Translate-URL: https://translation.odoo-community.org/projects/server-ux-11-0/server-ux-11-0-date_range/ar/

If any module adds a required field on company, module fails. Executing it on post_install, there's no problem.

Currently translated at 82.6% (38 of 46 strings) Translation: server-ux-11.0/server-ux-11.0-date_range Translate-URL: https://translation.odoo-community.org/projects/server-ux-11-0/server-ux-11-0-date_range/da/

espo-tony

I tested it and works fine. Code looks good too. 👍

date_range/models/date_range.py

lmignon · 2018-10-02T09:50:47Z

@astirpe Thank you for the migration. A little change to do otherwise LGTM

SimoRubi

Thanks for your contribution!
Please check the warning thrown by runbot too

SimoRubi · 2018-10-02T11:31:14Z

date_range/models/date_range.py

+        ('date_range_uniq', 'unique (name,type_id, company_id)',
+         'A date range must be unique per company !')]
+
+    @api.onchange('company_id')


should depend on type_id too

date_range/models/date_range_type.py

SimoRubi · 2018-10-02T11:38:15Z

date_range/models/date_range_type.py

+            for rec in self.sudo():
+                if not rec.company_id:
+                    continue
+                if bool(self.date_range_ids.filtered(


I think you don't need the cast to bool.
Check on rec instead of self.

@SimoRubi I think you misunderstand the code.....
We check if a date_range with another company lambda r: r.company_id and r.company_id != rec.company_id already exists into the list of date ranges (self.date_range_ids) with the same type.

Thanks for the clarification @lmignon, but if self is a recordset, then wouldn't self.date_range_ids throw an Exception?

@SimoRubi You're right, the logic here is not right.... may be something like

for rec in self.sudo(): company_ids = filtered(None, rec.date_range_ids.mapped('company_id')) if company_ids and company_ids != rec.company_id: raise ....

to be tested

But replacing self by rec is even more easy and readable ;-)

With latest commit, the self is replaced by rec. Is that enough? Do you all agree?

SimoRubi · 2018-10-02T11:39:09Z

date_range/models/date_range_type.py

+                    raise ValidationError(
+                        _('You cannot change the company, as this '
+                          'Date Range Type is  assigned to Date Range '
+                          '(%s).') % (self.date_range_ids.name_get()[0][1]))


Use rec instead of self.

@SimoRubi see my previous comment. This code is right.

tarteo · 2018-10-02T11:42:33Z

date_range/readme/INSTALL.rst

@@ -0,0 +1,3 @@
+The addon use the daterange method from postgres. This method is supported as of postgresql 9.2
+
+


Too many new lines.

date_range/static/src/js/date_range.js

tarteo · 2018-10-02T11:45:28Z

date_range/static/src/js/date_range.js

+    },
+
+    get_value: function () {
+        return parseInt(this.$el.val());


Can you specify the radix.

date_range/static/src/js/date_range.js

tarteo · 2018-10-02T11:48:50Z

date_range/static/src/js/date_range.js

+
+    date_range_type_operator_selected: function (type_id){
+        this.$value.empty().show();
+        var ds = new data.DataSetSearch(this, 'date.range', this.context, [['type_id', '=', parseInt(type_id)]]);


Can you specify the radix?

SimoRubi · 2018-10-02T11:58:03Z

@astirpe also note that the folder migrations has to be removed as stated in https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-12.0#tasks-to-do-in-the-migration:

Remove any possible migration script from previous version.

astirpe · 2018-10-02T12:20:52Z

@SimoRubi I think that the actual migration script (11.0.2.0.0) should be kept as it is, to handle the case of migration directly from 11.0.1.0.0 to 12.0.1.0.0.

pedrobaeza · 2018-10-02T12:43:47Z

@astirpe No, the rule is to always be on latest updated version of N-1 module for migrating to N. If not, you will get a lot of side effects. Trust me on this one 😉

SimoRubi

ok for me

SimoRubi

Please check runbot

astirpe · 2018-10-02T13:08:28Z

Migration scripts are removed.
The string="Type Name" is restored.
Latest commits are squashed.

lmignon

@astirpe It could be nice to backport the fix into the date_range_type constrains in previous version....

tarteo

Thanks!

SimoRubi

Ok for me

astirpe · 2018-10-02T13:58:56Z

The code review done within this PR is backported to V11: #26

This was referenced Oct 1, 2018

Migration to version 12.0 #24

Closed

[12.0][MIG] account_tax_balance OCA/account-financial-reporting#468

Merged

pedrobaeza added this to the 12.0 milestone Oct 2, 2018

lmignon and others added 21 commits October 2, 2018 10:36

OCA Transbot updated translations from Transifex

fc0b579

[10.0][MIGRATION] Migrated module 'date_range' to V10

fddf1dd

OCA Transbot updated translations from Transifex

00b05cc

[REF] date_range: Use 'index' instead of 'select'

0d1feaf

OCA Transbot updated translations from Transifex

6bcbd0f

[IMP] date_range: Several things

1f18334

* Improve 'name' for generator wizard ir.rule should be active by default * Don't auto-add '-' after prefix when generating date ranges via wizard * code fine tuning suggested by Sylvain Garancher

OCA Transbot updated translations from Transifex

6650a79

[10.0][FIX][date_range] Periods of one day should be allowed (#947)

4a54a1e

[MIG] date_range

a44c13a

[MIG] date_range: Adapt JS to new framework

9616d59

Fix unlink date range type.

c31ed03

[IMP] date_range: adapt to multicompany

d4fa110

[IMP] date_range: new README structure

e658014

[UPD] README.rst

1075227

[UPD] Update date_range.pot

0ce4643

Translated using Weblate (Arabic)

8e6776f

Currently translated at 89.1% (41 of 46 strings) Translation: server-ux-11.0/server-ux-11.0-date_range Translate-URL: https://translation.odoo-community.org/projects/server-ux-11-0/server-ux-11-0-date_range/ar/

[IMP] date_range: Pass integration tests

dde45ee

If any module adds a required field on company, module fails. Executing it on post_install, there's no problem.

[UPD] README.rst

c42963a

[UPD] Update date_range.pot

55f9091

Translated using Weblate (Danish)

d96646d

Currently translated at 82.6% (38 of 46 strings) Translation: server-ux-11.0/server-ux-11.0-date_range Translate-URL: https://translation.odoo-community.org/projects/server-ux-11-0/server-ux-11-0-date_range/da/

espo-tony approved these changes Oct 2, 2018

View reviewed changes

lmignon requested changes Oct 2, 2018

View reviewed changes

date_range/models/date_range.py Show resolved Hide resolved

SimoRubi suggested changes Oct 2, 2018

View reviewed changes

tarteo suggested changes Oct 2, 2018

View reviewed changes

SimoRubi approved these changes Oct 2, 2018

View reviewed changes

SimoRubi suggested changes Oct 2, 2018

View reviewed changes

ThomasBinsfeld approved these changes Oct 2, 2018

View reviewed changes

lmignon approved these changes Oct 2, 2018

View reviewed changes

tarteo approved these changes Oct 2, 2018

View reviewed changes

SimoRubi approved these changes Oct 2, 2018

View reviewed changes

astirpe mentioned this pull request Oct 2, 2018

[11.0][FIX] date_range: fix constrains + fix onchange #26

Merged

5 tasks

mpanarin approved these changes Oct 2, 2018

View reviewed changes

astirpe added 2 commits October 2, 2018 17:47

[12.0][MIG] date_range

a40a8dd

eslint

efe409b

pedrobaeza merged commit a81479c into OCA:12.0 Oct 4, 2018

astirpe deleted the 12_mig_date_range branch October 4, 2018 08:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[12.0][MIG] date_range #25

[12.0][MIG] date_range #25

astirpe commented Oct 1, 2018

espo-tony left a comment

lmignon commented Oct 2, 2018

SimoRubi left a comment

SimoRubi Oct 2, 2018

SimoRubi Oct 2, 2018

lmignon Oct 2, 2018

SimoRubi Oct 2, 2018

lmignon Oct 2, 2018

lmignon Oct 2, 2018

astirpe Oct 2, 2018

SimoRubi Oct 2, 2018

lmignon Oct 2, 2018

tarteo Oct 2, 2018

tarteo Oct 2, 2018

tarteo Oct 2, 2018

SimoRubi commented Oct 2, 2018

astirpe commented Oct 2, 2018

pedrobaeza commented Oct 2, 2018

SimoRubi left a comment

SimoRubi left a comment

astirpe commented Oct 2, 2018

lmignon left a comment

tarteo left a comment

SimoRubi left a comment

astirpe commented Oct 2, 2018

		@@ -0,0 +1,3 @@
		The addon use the daterange method from postgres. This method is supported as of postgresql 9.2

[12.0][MIG] date_range #25

[12.0][MIG] date_range #25

Conversation

astirpe commented Oct 1, 2018

espo-tony left a comment

Choose a reason for hiding this comment

lmignon commented Oct 2, 2018

SimoRubi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SimoRubi commented Oct 2, 2018

astirpe commented Oct 2, 2018

pedrobaeza commented Oct 2, 2018

SimoRubi left a comment

Choose a reason for hiding this comment

SimoRubi left a comment

Choose a reason for hiding this comment

astirpe commented Oct 2, 2018

lmignon left a comment

Choose a reason for hiding this comment

tarteo left a comment

Choose a reason for hiding this comment

SimoRubi left a comment

Choose a reason for hiding this comment

astirpe commented Oct 2, 2018