[9.0][MIG] medical #69

lasley · 2016-01-31T22:07:48Z

I also changed the License to markdown version - looks nicer IMO

pedrobaeza · 2016-02-01T06:49:28Z

I'm not sure if the license file should be changed as is a verbatim copy of the one provided by the FSF...

lasley · 2016-02-02T04:25:01Z

@pedrobaeza No problem, updated

JayVora-SerpentCS · 2016-02-03T08:31:05Z

medical/README.rst

+`here <https://github.com/OCA/vertical-medical/issues/new?body=module:%20medical%0Aversion:%209.0.1.1.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_.
+=======
+`here <https://github.com/OCA/vertical-medical/issues/new?body=module:%20medical%0Aversion:%209.0.2.0.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_.
+>>>>>>> feature/SMD-88-upgrade-allergy-support-in-medical.-9.0



Seems conflicted.

pedrobaeza · 2016-02-04T07:16:39Z

medical/README.rst

@@ -71,7 +71,7 @@ Bug Tracker
 Bugs are tracked on `GitHub Issues <https://github.com/OCA/vertical-medical/issues>`_.
 In case of trouble, please check there if your issue has already been reported.
 If you spotted it first, help us smashing it by providing a detailed and welcomed feedback
-`here <https://github.com/OCA/vertical-medical/issues/new?body=module:%20medical%0Aversion:%208.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_.
+`here <https://github.com/OCA/vertical-medical/issues/new?body=module:%20medical%0Aversion:%209.0.2.0.0%0A%0A**Steps%20to%20reproduce**%0A-%20...%0A%0A**Current%20behavior**%0A%0A**Expected%20behavior**>`_.


Please put here only branch (9.0)

pedrobaeza · 2016-02-04T07:22:09Z

I have made a rough code review. You're removing entirely some code and views, is this moved to another module?

lasley · 2016-02-04T07:25:47Z

Thanks @pedrobaeza -

Will apply the recommended fixes (and will apply recommendations to the other PRs where applicable).

All of the medical_appointment logic was moved to two separate modules (sorry I should have noted this in the comments of the PR):

medical_base_history - Contains the a slightly upgraded/more abstract form of the change tracking used in medical_appointment
medical_appointment - The actual appointment logic

Both of these are in #70

lasley · 2016-02-10T21:20:42Z

Hi all - I added a rough dependency chain in the wiki to help with reviewing of these PRs. Wasn't sure the best approach to take, so I went with text based- https://github.com/OCA/vertical-medical/wiki#dependency-chain

coveralls · 2016-06-10T21:21:08Z

Changes Unknown when pulling 9821ec3 on laslabs:feature/9.0-medical into * on OCA:9.0*.

coveralls · 2016-06-10T21:24:39Z

Changes Unknown when pulling 9821ec3 on laslabs:feature/9.0-medical into * on OCA:9.0*.

…227-fix-prescription-order-line-kanban to release/9.0 * commit 'd6ae738f98160b52eced61def08812118c50fcf5': [FIX] medical_prescription: Fix search view ref in order_line action.

…ents plus some other small additions and changes.

…me medical_patient model compute methods.

lasley · 2016-09-25T18:42:32Z

This has been rebased to include all changes in our v9 medical & is now ready for review

lasley · 2016-09-26T23:11:22Z

medical/medical_constants.py

-##############################################################################
+# Copyright 2004 Tech-Receptives
+# Copyright 2016 LasLabs Inc.
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).

 months = {
    1: "January", 2: "February", 3: "March", 4: "April",


Please add translations to these strings

Fixed. Added translations to months of the year and days of the week.

Isn't this month list in any place across Odoo?

I have no idea TBH. This is leftover from legacy & I'm not even 100% where it's all being used. We'll probably remove entirely in v10, I just haven't had time to audit fully

OK, let's move this way for now.

@BMW95 - Please add a note in the ReadMe roadmap regarding the removal of medical constants

Is this something similar? After some searching I noticed the time_intervals dictionary (on line 1943) used in several base_ addons.

Couple addons I noticed using the months etc from medical (medical_physician being one).

Do you guys want me to add this to the roadmap section in the readme?

oh my bad sorry @lasley didn't catch your comment in time

Nope time_intervals are deltas for using w/ math. These are meant for select fields (I think).

lasley · 2016-09-26T23:15:36Z

medical/models/medical_patient.py

+    gender = fields.Selection([
+        ('m', 'Male'),
+        ('f', 'Female'),
+    ], )


Putting the ) on the same line as the ] defeats the purpose of the trailing ,, which is to allow for addition of attributes without changing unrelated lines of code. Should be:

fields.Selection([ ... ], )

@lasley Getting lint errors from that way. Can I do:

fields.Selection( [ ... ], )

?

lasley · 2016-09-26T23:15:43Z

medical/models/medical_patient.py

+        ('d', 'Divorced'),
+        ('x', 'Separated'),
+        ('z', 'law marriage'),
+    ], )


Putting the ) on the same line as the ] defeats the purpose of the trailing ,, which is to allow for addition of attributes without changing unrelated lines of code. Should be:

fields.Selection([ ... ], )

lasley · 2016-09-26T23:16:38Z

medical/models/medical_patient.py

+                    delta.days, _('d'), deceased
+                )
+            else:
+                years_months_days = _('No DoB !')


Remove the space between DoB and !

Done. Tests updated for that.

lasley · 2016-09-26T23:17:49Z

medical/models/medical_patient.py

+    def _compute_deceased(self):
+        for rec_id in self:
+            if rec_id.dod:
+                rec_id.deceased = True


rec_id.deceased = bool(rec_id.dod)

aw nice man. Knew I was doing it a bit clunky, forget sometimes about that approach. Done.

lasley · 2016-09-26T23:19:07Z

medical/models/res_partner.py

    )
    last_name = fields.Char(
        string='Last Name',
        size=256,
-        help='Last Name',
+        help='Last name of the party',


s/party/partner

Field removed as per comment above.

lasley · 2016-09-26T23:20:49Z

medical/models/res_partner.py

-        string='Activation date',
-        help='Date of activation of the party',
+        string='Activation Date',
+        help='Date the party was activated',
    )
    last_name = fields.Char(


We should remove this - it's not used anywhere & will create confusion.

Please add isolation of first and last names into the roadmap as well. It should probably be in patient, not partner too.

Removed. Added "Separation of patient/res_partner name into first and last names." to roadmap in readme.

lasley · 2016-09-26T23:21:00Z

medical/models/res_partner.py

+    )
+    is_work = fields.Boolean(
+        string='Work',
+        help='Check if the party is a place of work',


s/party/partner

lasley · 2016-09-26T23:21:04Z

medical/models/res_partner.py

    is_person = fields.Boolean(
        string='Person',
-        help='Check if the party is a person.',
+        help='Check if the party is a person',


s/party/partner

No need to note done on each of the items. You only need comment on them if you have a question, comment, etc. When fully done, just drop a global comment advising of such

lasley · 2016-09-26T23:22:40Z

medical/models/res_partner.py

-        help='Check if the party is a person.',
+        help='Check if the party is a person',
+    )
+    is_school = fields.Boolean(


Add into roadmap (for v10):

Remove is_* attributes on in favor of res.partner.type model & a M2M relation

Added: "(Odoo v10) Remove all is_* fields (e.g. is_school, is_work, etc.) from res.partner and move to res.partner.type."

Visually grouped all the is_* fields together in the res.partner.py file

lasley · 2016-09-26T23:23:57Z

@BMW95 - comments inline. Side note - Those tests look so sexy now that you're using the demo data 🎉

…c. Add items to roadmap in readme.

woodbrettm · 2016-09-27T22:08:22Z

Ready for further review :)

lasley · 2016-09-27T23:00:14Z

medical/models/res_partner.py

    )
    is_person = fields.Boolean(
        string='Person',
-        help='Check if the party is a person',
+        help='Check if the s/party/partner is a person',


Lmao s/party/partner means spelling error. Change word(s) on left side with words on right. This should say partner

oh hahahaha. Yeah wasn't quite sure what that meant.

Ask in chat next time 😉

lasley · 2016-09-27T23:39:09Z

Thanks @BMW95 - LGTM 👍

lasley · 2016-09-29T00:13:08Z

@t3ddftw please review when you get a chance.

tedsalmon · 2016-10-11T07:19:53Z

medical/models/res_partner.py

    relationship = fields.Char(
-        'Relationship',
+        string='Relationship',


This is superfluous given the field name.

tedsalmon · 2016-10-11T07:28:25Z

LGTM aside from the minor detail I noted. 👍

…s the same as the string in res.partner.

* [MIG] Upgrade medical to 9.0. * [FIX] medical: Fix file headers, add social insurance numbers to patients plus some other small additions and changes. * [IMP] medical: Refactor tests to use demo data, add api.depends to some medical_patient model compute methods. * [FIX] medical: Various adjustments to field definitions, patient logic. Add items to roadmap in readme. * [FIX] medical: Fix help descriptions in res.partner file. * [FIX] medical: Remove string attributes where the name of the field is the same as the string in res.partner.

lasley force-pushed the feature/9.0-medical branch from d3b8ec6 to 953116f Compare January 31, 2016 22:10

This was referenced Jan 31, 2016

[9.0][MIG] medical appointment #70

Closed

[9.0][MIG] medical pathology #71

Merged

lasley force-pushed the feature/9.0-medical branch from d575380 to 953116f Compare January 31, 2016 22:35

This was referenced Jan 31, 2016

[9.0][MIG] medical medicament #74

Merged

[9.0][MIG] medical pharmacy #77

Merged

lasley changed the title ~~Upgrade medical to v9 & add tests~~ Feature/9.0-medical Jan 31, 2016

lasley changed the title ~~Feature/9.0-medical~~ Feature/9.0 medical Jan 31, 2016

JayVora-SerpentCS reviewed Feb 3, 2016
View reviewed changes

lasley mentioned this pull request Feb 4, 2016

Migrated modules to v9 #82

Closed

pedrobaeza mentioned this pull request Feb 4, 2016

Migration to version 9.0 #50

Closed

30 tasks

pedrobaeza reviewed Feb 4, 2016
View reviewed changes

lasley force-pushed the feature/9.0-medical branch from 3572b9d to 7930f14 Compare February 4, 2016 08:57

abigailfoster mentioned this pull request Feb 4, 2016

[9.0][MIG] medical patient dob #84

Merged

1 task

kenwmak force-pushed the feature/9.0-medical branch 3 times, most recently from b798888 to 7930f14 Compare February 10, 2016 08:40

This was referenced Feb 10, 2016

[9.0][MIG] medical physician #86

Merged

[9.0][MIG] medical physician us #87

Closed

lasley force-pushed the feature/9.0-medical branch 2 times, most recently from e026166 to 9821ec3 Compare June 10, 2016 21:19

lasley added the work in progress label Aug 15, 2016

lasley added this to the 9.0 milestone Aug 15, 2016

Brett Wood added 3 commits September 19, 2016 11:50

[MIG] Upgrade medical to 9.0.

bbb89fb

[FIX] medical: Fix file headers, add social insurance numbers to pati…

26e311b

…ents plus some other small additions and changes.

[IMP] medical: Refactor tests to use demo data, add api.depends to so…

8c2f92e

…me medical_patient model compute methods.

lasley force-pushed the feature/9.0-medical branch from 9821ec3 to 8c2f92e Compare September 25, 2016 18:41

lasley added needs review and removed work in progress labels Sep 25, 2016

lasley commented Sep 26, 2016

View reviewed changes

[FIX] medical: Various adjustments to field definitions, patient logi…

8e1ac5a

…c. Add items to roadmap in readme.

woodbrettm force-pushed the feature/9.0-medical branch from 79950d5 to 8e1ac5a Compare September 27, 2016 21:44

lasley commented Sep 27, 2016

View reviewed changes

[FIX] medical: Fix help descriptions in res.partner file.

9ad6f40

lasley changed the title ~~Feature/9.0 medical~~ [9.0][MIG] medical Sep 28, 2016

tedsalmon reviewed Oct 11, 2016

View reviewed changes

medical/models/res_partner.py

relationship = fields.Char(

'Relationship',

string='Relationship',

Copy link

tedsalmon Oct 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is superfluous given the field name.

[FIX] medical: Remove string attributes where the name of the field i…

7dcb389

…s the same as the string in res.partner.

lasley merged commit 56c6757 into OCA:9.0 Oct 14, 2016

lasley deleted the feature/9.0-medical branch October 14, 2016 21:37

[9.0][MIG] medical #69

[9.0][MIG] medical #69

Conversation

lasley commented Jan 31, 2016

pedrobaeza commented Feb 1, 2016

lasley commented Feb 2, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pedrobaeza commented Feb 4, 2016

lasley commented Feb 4, 2016

lasley commented Feb 10, 2016

coveralls commented Jun 10, 2016 • edited Loading

coveralls commented Jun 10, 2016 • edited Loading

lasley commented Sep 25, 2016 • edited Loading

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

lasley Sep 27, 2016 • edited Loading

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

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

lasley commented Sep 26, 2016

woodbrettm commented Sep 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lasley commented Sep 27, 2016

lasley commented Sep 29, 2016

Choose a reason for hiding this comment

tedsalmon commented Oct 11, 2016

coveralls commented Jun 10, 2016 •

edited

Loading

coveralls commented Jun 10, 2016 •

edited

Loading

lasley commented Sep 25, 2016 •

edited

Loading

lasley Sep 27, 2016 •

edited

Loading