OCA / fleet

Odoo fleet management modules
GNU Affero General Public License v3.0
29 stars 79 forks source link

[ADD][14.0] Add vehicle ownership, linking partners to vehicles #127 #130

Closed rpsjr closed 1 year ago

rpsjr commented 1 year ago

127

126

This PR Add vehicle ownership, linking partners to vehicles.

cc @ivantodorovich @mamcode @sbidoul @mymage @brian10048 @marcelsavegnago @pedrobaeza

etobella commented 1 year ago

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 [ADD] ...

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 😉

rpsjr commented 1 year ago

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 [ADD] ...

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 😉

done

etobella commented 1 year ago

Commit message should be [ADD] fleet_vehicle_ownership

Add some tests please

rpsjr commented 1 year ago

Commit message should be [ADD] fleet_vehicle_ownership

Add some tests please

done!

etobella commented 1 year ago

Once the changes are applied, we can merge. Thanks for the great job :smile:

rpsjr commented 1 year ago

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 https://github.com/OCA/fleet/pull/130#discussion_r1404420433:

  • 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 https://github.com/OCA/fleet/pull/130#discussion_r1404421079:

  • 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 https://github.com/OCA/fleet/pull/130#pullrequestreview-1748056581, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKCBYEPUWHJWQTHP42QUD3TYGCWP5AVCNFSM6AAAAAA7YIXFQOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTONBYGA2TMNJYGE . You are receiving this because you were mentioned.Message ID: @.***>

etobella commented 1 year ago

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

rpsjr commented 1 year ago

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 https://github.com/OCA/fleet/pull/130#issuecomment-1825931654, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKCBYEJ62YFK5KFZ775TZ2DYGDIORAVCNFSM6AAAAAA7YIXFQOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRVHEZTCNRVGQ . You are receiving this because you were mentioned.Message ID: @.***>

etobella commented 1 year ago

So, we can proceed for merge.

Thanks for doing the all the changes :wink:

/ocabot merge nobump

OCA-git-bot commented 1 year ago

Hey, thanks for contributing! Proceeding to merge this for you. Prepared branch 14.0-ocabot-merge-pr-130-by-etobella-bump-nobump, awaiting test results.

OCA-git-bot commented 1 year ago

Congratulations, your PR was merged at 9294ddcb47f364d7b3995cad1dadcdab4a4b69da. Thanks a lot for contributing to OCA. ❤️