-
-
Notifications
You must be signed in to change notification settings - Fork 81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ADD][14.0] Add vehicle ownership, linking partners to vehicles #127 #130
Conversation
Can you squaah your commits in a single one? You can use git rebate in order to do that. Also fix the commit name to Last, but not least, you don't need to close and open a new issue. If you use git rebase, you can do it all at once 😉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for models and views. I don't know if all the file in the root (as .gitignore) are right bat this is a my limit :-(.
94e3930
to
ceedfa5
Compare
done |
Commit message should be Add some tests please |
0f07089
to
3050230
Compare
e3fa333
to
32de3e1
Compare
8127d96
to
239c4f8
Compare
done! |
f11c934
to
56be8d2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rpsjr Congratulations on the work and thank you very much for your contribution.
) | ||
|
||
def action_view_vehicles(self): | ||
xmlid = "fleet.fleet_vehicle_action" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should add self.ensure_one()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
def action_view_vehicles(self): | ||
xmlid = "fleet.fleet_vehicle_action" | ||
action = self.env["ir.actions.act_window"]._for_xml_id(xmlid) | ||
if self.vehicle_count > 1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recomend to add a default_owner_id = self.id on context
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Once the changes are applied, we can merge. Thanks for the great job 😄 |
What about default owner beeing the company (res.partner) as this is
implicit when fleet app is used to HR purooses?
…On Fri, Nov 24, 2023, 11:38 Enric Tobella ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In fleet_vehicle_ownership/models/res_partner.py
<#130 (comment)>:
> + def _compute_vehicle_count(self):
+ for rec in self:
+ rec.vehicle_count = len(rec.vehicle_ids)
+
+ vehicle_ids = fields.One2many(
+ "fleet.vehicle",
+ "owner_id",
+ required=True,
+ help="Vehicles owned by this partner",
+ )
+ vehicle_count = fields.Integer(
+ compute=_compute_vehicle_count, string="Number of Vehicles", store=True
+ )
+
+ def action_view_vehicles(self):
+ xmlid = "fleet.fleet_vehicle_action"
You should add self.ensure_one()
------------------------------
In fleet_vehicle_ownership/models/res_partner.py
<#130 (comment)>:
> + rec.vehicle_count = len(rec.vehicle_ids)
+
+ vehicle_ids = fields.One2many(
+ "fleet.vehicle",
+ "owner_id",
+ required=True,
+ help="Vehicles owned by this partner",
+ )
+ vehicle_count = fields.Integer(
+ compute=_compute_vehicle_count, string="Number of Vehicles", store=True
+ )
+
+ def action_view_vehicles(self):
+ xmlid = "fleet.fleet_vehicle_action"
+ action = self.env["ir.actions.act_window"]._for_xml_id(xmlid)
+ if self.vehicle_count > 1:
I would recomend to add a default_owner_id = self.id on context
—
Reply to this email directly, view it on GitHub
<#130 (review)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKCBYEPUWHJWQTHP42QUD3TYGCWP5AVCNFSM6AAAAAA7YIXFQOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTONBYGA2TMNJYGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, what I am saying is that you are on a partner and clicking the vehicles. From there you can add a new one. Odoo logic says that you are adding a vehicle to the original partner. So it makes sense to do as I say |
Gotta, I misunderstood the previous msg.
…On Fri, Nov 24, 2023, 14:12 Enric Tobella ***@***.***> wrote:
What about default owner beeing the company (res.partner) as this is
implicit when fleet app is used to HR purooses?
Well, what I am saying is that you are on a partner and clicking the
vehicles. From there you can add a new one. Odoo logic says that you are
adding a vehicle to the original partner. So it makes sense to do as I say
—
Reply to this email directly, view it on GitHub
<#130 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKCBYEJ62YFK5KFZ775TZ2DYGDIORAVCNFSM6AAAAAA7YIXFQOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRVHEZTCNRVGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
3a5752f
to
cc7a598
Compare
rfc rfc clean up [RFC] cleanup redundant files pre-commit [RFC] improve res_partner pre-commit typo typo typo [RFC] default_owner_id [RFC] tests
e449aa4
to
72590be
Compare
So, we can proceed for merge. Thanks for doing the all the changes 😉 /ocabot merge nobump |
Hey, thanks for contributing! Proceeding to merge this for you. |
Congratulations, your PR was merged at 9294ddc. Thanks a lot for contributing to OCA. ❤️ |
#127
#126
This PR Add vehicle ownership, linking partners to vehicles.
cc @ivantodorovich @mamcode @sbidoul @mymage @brian10048 @marcelsavegnago @pedrobaeza