[8.0] Refactor lettermgmt #87

ivantodorovich · 2016-03-17T01:48:23Z

As discussed in #83

Feel free to comment and make suggestions as I work.

TODO:

~~Migrate field class to class_id~~ migrated to category_ids.
~~Add seccurity group to show/hide reassignments~~
~~Add seccurity group to show/hide partner / child~~
~~Increment sequence only after the record is saved, not before.~~
~~Simplify and improve state workflow.~~
~~Support current "advanced" workflow in a separate module.~~ wont support it in this pr
~~Add default filter 'Pending' and search fields to list views.~~
~~Add tests~~

hbrunn · 2016-03-17T04:58:19Z

another great thing to add would be tests...

coveralls · 2016-03-19T23:40:01Z

Coverage increased (+0.2%) to 78.748% when pulling 3d076ef on ivantodorovich:8.0-ref-lettermgmt into e8f978b on OCA:8.0.

coveralls · 2016-03-22T00:40:53Z

Coverage increased (+1.5%) to 80.12% when pulling 3859eb0 on ivantodorovich:8.0-ref-lettermgmt into e8f978b on OCA:8.0.

coveralls · 2016-03-22T00:43:07Z

Coverage increased (+1.5%) to 80.12% when pulling 3859eb0 on ivantodorovich:8.0-ref-lettermgmt into e8f978b on OCA:8.0.

coveralls · 2016-03-22T22:21:44Z

Coverage increased (+4.9%) to 83.434% when pulling db3f170 on ivantodorovich:8.0-ref-lettermgmt into e8f978b on OCA:8.0.

coveralls · 2016-03-22T22:31:54Z

Coverage increased (+5.3%) to 83.886% when pulling 9553619 on ivantodorovich:8.0-ref-lettermgmt into e8f978b on OCA:8.0.

ivantodorovich · 2016-03-22T23:08:02Z

@hbrunn
Please take a look when you have the time.

Functional changes:

Field class is now a Many2many_tags called category_ids
Field date is now field.Date (instead of Datetime).
Tree View improvements
- Added colours
- Added a default filter 'Pending' to show letters that are not received yet (either by us or by the external partner)
The workflow for in and out letters is now the same.
- This way we can track letters sent until we get confrimation of reception and manually set it to 'received'
- snd_rec_date is replaced by fields snd_date and rec_date
- snd_date is the date the letter was sent either by us (out) or by the external sender (in)
- rec_date is the date the letter was received by us (in) or the date the recipient confirmed (out)
Simplified workflow
- Removed created and validated states.
- Current workflow is:
  - Draft -> Sent -> Received / Rec. Returned / Rec. Bad
  - Cancelled -> Back to Draft
Groups are added to show/hide some fields for extended usage
- group to show/hide parent and childs ("thread")
- group to show/hide reassignments
Sender and Recipient are now defaulted to current company, depending on the letter move (in or out)
It no longer add subscribers when changing the state. To my understanding, it was annoying.
Some other changes..

ivantodorovich · 2016-03-22T23:12:52Z

I'm need to make the module to extend the workflow. It will re-implement the 'created' and 'validated' states.

I still don't completely understand the use case, though.. I mean, it didn't have security groups, so any user was able to create or validate any letter. If it was only to track the letter prep states before sending, the same could be achived by just using internal notes or messages.

Even so, it seems to be a very company-specific case. I think it's very possible no one will use it anymore (and ever).

Should I do it the same? I don't mind.. I just don't like the idea of useless modules.

ivantodorovich · 2016-03-22T23:24:15Z

@max3903 I'd like to hear your thoughts too

hbrunn · 2016-09-13T06:59:00Z

what's the state of this PR? Can we review?

hbrunn · 2016-09-13T07:02:37Z

(asking because I'm in the process of adding a 'done' state and support for needaction, and it would make sense to get your refactoring merged first of course)

hbrunn · 2016-09-13T07:16:40Z

lettermgmt/migrations/8.0.2.0/pre-migration.py

+
+    # Migrate 'type' to 'type_id'
+    cr.execute("""ALTER TABLE res_letter
+                  RENAME COLUMN type to type_id""")


you should borrow openupgrade's code for renaming a model here: https://github.com/OCA/openupgradelib/blob/master/openupgradelib/openupgrade.py#L328 to get links from other models (mostly) right after you renamed letter.class

ivantodorovich · 2016-09-14T00:11:21Z

@hbrunn
what's the state of this PR? Can we review?
(asking because I'm in the process of adding a 'done' state and support for needaction, and it would make sense to get your refactoring merged first of course)

Well, I kinda stop working on it because of the low attention it got.
But I believe this PR was almost ready, except for the 'advanced workflow' support, which is removed and it should be implemented in a separate module because of its rare usage.. but maybe no one will ever use it.

I'll take a look at openupgrade's code, as you suggested, and make the changes later this week.

hbrunn · 2016-09-14T04:44:57Z

that's probably because of the work in progress label - I changed this to needs review to invite other reviewers

hbrunn · 2016-09-14T04:49:44Z

lettermgmt/models/res_letter.py

+        """ Go from cancelled state to draf state """
+        self.write({'state': 'draft'})
+        self.delete_workflow()
+        self.create_workflow()


this model doesn't define a workflow (in the sense of odoo's workflows), so why do you need this?

At the time of this PR, I was just starting to learn odoo. I guess I've just copied this from another module's code, and it doesn't apply here at all.

ivantodorovich · 2016-09-16T02:00:53Z

I need help with migration. I'm not happy with how it is right now. I'm not sure if it works properly either.

hbrunn · 2016-09-16T17:08:10Z

@ivantodorovich just from looking at the code, the migration seems okay. What is it you're not happy about?

ivantodorovich · 2016-09-16T18:10:36Z

This right here:

cr.execute("""INSERT INTO letter_category SELECT * FROM letter_class""")

openupgrade renames the table instead of copying all the records from the old table into the new table.
I tried to do it that way, in pre-migration, but odoo won't accept it for some reason I couldn't figure out.

Anyway, considering this module's popularity, I guess no one will ever care about that, as long as it works.

hbrunn · 2016-09-16T18:55:55Z

what error message do you get here? And did you prepare your test database accordingly?

rafaelbn · 2016-10-04T15:30:58Z

Do you need any functional help here @ivantodorovich @hbrunn ?

ivantodorovich · 2016-10-04T16:53:38Z

I'm sorry for not responding earlier.
I haven't been able to make up the time this week.

I'll test again the migration code on a clean database and see if I can improve it.
Any help reviewing the migration would be great.

pedrobaeza · 2016-10-13T23:30:28Z

This PR is hard to review, but I can trust @hbrunn if he says this is OK

pedrobaeza · 2016-10-18T21:13:39Z

@ivantodorovich, can you squash a bit the commits?

@hbrunn, any comments?

ivantodorovich · 2016-10-19T01:04:11Z

Done. Squashed into a single commit.

hbrunn · 2016-10-19T06:13:53Z

my comment is let's merge as soon as @ivantodorovich has fixed the last travis nags: https://travis-ci.org/OCA/crm/jobs/168795594#L314

hbrunn · 2016-10-19T06:15:03Z

better link: https://travis-ci.org/OCA/crm/jobs/168795594#L307-L323

Updated py headers. Added headers to xml files. Reordered res_letter.py lines to make them more readable. Improved help strings on a few fields. Added default values for sender/recipient. Simplified views. Unified 'in' and 'out' letter views. Major view improvements. Changed class to category_ids (Many2many_tags). Changed type to type_id (for consistency). Added migration script. Added view's 'oe_view_nocontent_create' helps for types, categories, channels and folders. Add security group to hide/show reassignments and parent/child. Increment sequence only after the record is saved, not before. Simplify and improve workflow. Known Issue: 'created' and 'validated' states are reverted to 'draft'. Added search filter 'Pending' and set by default (shows letters that haven't arrived to destination). Make 'recipient' and 'sender' fields required in the model, instead of required only in the view. Add tests

ivantodorovich · 2016-10-20T00:01:56Z

Done, sorry about that!

pedrobaeza · 2016-10-20T00:38:44Z

Thanks for your efforts!

hbrunn added the work in progress label Mar 17, 2016

hbrunn added this to the 8.0 milestone Mar 17, 2016

hbrunn reviewed Sep 13, 2016
View reviewed changes

hbrunn added needs review and removed work in progress labels Sep 14, 2016

hbrunn reviewed Sep 14, 2016
View reviewed changes

hbrunn changed the title ~~[8.0][WIP] Refactor lettermgmt~~ [8.0] Refactor lettermgmt Sep 14, 2016

hbrunn mentioned this pull request Sep 29, 2016

[MIG] crm:lettermgmt Migrated Module to V9. #112

Closed

ivantodorovich mentioned this pull request Oct 16, 2016

[MIG][9.0] lettermgmt: Migrated module, created this new PR as mentioned in PR #112. #115

Closed

pedrobaeza merged commit c0231f3 into OCA:8.0 Oct 20, 2016

hbrunn mentioned this pull request Oct 31, 2016

[IMP] rename table instead of copying #121

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[8.0] Refactor lettermgmt #87

[8.0] Refactor lettermgmt #87

ivantodorovich commented Mar 17, 2016 •

edited

Loading

hbrunn commented Mar 17, 2016

coveralls commented Mar 19, 2016

coveralls commented Mar 22, 2016

coveralls commented Mar 22, 2016

coveralls commented Mar 22, 2016

coveralls commented Mar 22, 2016

ivantodorovich commented Mar 22, 2016

ivantodorovich commented Mar 22, 2016

ivantodorovich commented Mar 22, 2016

hbrunn commented Sep 13, 2016

hbrunn commented Sep 13, 2016

hbrunn Sep 13, 2016

ivantodorovich Sep 16, 2016

ivantodorovich commented Sep 14, 2016

hbrunn commented Sep 14, 2016

hbrunn Sep 14, 2016

ivantodorovich Sep 14, 2016

ivantodorovich commented Sep 16, 2016

hbrunn commented Sep 16, 2016

ivantodorovich commented Sep 16, 2016

hbrunn commented Sep 16, 2016

rafaelbn commented Oct 4, 2016

ivantodorovich commented Oct 4, 2016

pedrobaeza commented Oct 13, 2016

pedrobaeza commented Oct 18, 2016

ivantodorovich commented Oct 19, 2016

hbrunn commented Oct 19, 2016

hbrunn commented Oct 19, 2016

ivantodorovich commented Oct 20, 2016 •

edited

Loading

pedrobaeza commented Oct 20, 2016

[8.0] Refactor lettermgmt #87

[8.0] Refactor lettermgmt #87

Conversation

ivantodorovich commented Mar 17, 2016 • edited Loading

hbrunn commented Mar 17, 2016

coveralls commented Mar 19, 2016

coveralls commented Mar 22, 2016

coveralls commented Mar 22, 2016

coveralls commented Mar 22, 2016

coveralls commented Mar 22, 2016

ivantodorovich commented Mar 22, 2016

ivantodorovich commented Mar 22, 2016

ivantodorovich commented Mar 22, 2016

hbrunn commented Sep 13, 2016

hbrunn commented Sep 13, 2016

hbrunn Sep 13, 2016

Choose a reason for hiding this comment

ivantodorovich Sep 16, 2016

Choose a reason for hiding this comment

ivantodorovich commented Sep 14, 2016

hbrunn commented Sep 14, 2016

hbrunn Sep 14, 2016

Choose a reason for hiding this comment

ivantodorovich Sep 14, 2016

Choose a reason for hiding this comment

ivantodorovich commented Sep 16, 2016

hbrunn commented Sep 16, 2016

ivantodorovich commented Sep 16, 2016

hbrunn commented Sep 16, 2016

rafaelbn commented Oct 4, 2016

ivantodorovich commented Oct 4, 2016

pedrobaeza commented Oct 13, 2016

pedrobaeza commented Oct 18, 2016

ivantodorovich commented Oct 19, 2016

hbrunn commented Oct 19, 2016

hbrunn commented Oct 19, 2016

ivantodorovich commented Oct 20, 2016 • edited Loading

pedrobaeza commented Oct 20, 2016

ivantodorovich commented Mar 17, 2016 •

edited

Loading

ivantodorovich commented Oct 20, 2016 •

edited

Loading