Skip to content
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

[12.0][ADD] add fleet_vehicle_pivot_graph #56

Merged

Conversation

marcelsavegnago
Copy link
Member

No description provided.

@ivantodorovich
Copy link

I'm replying to your comment #51 (comment)

Yes, if I create a module to add only the pivot view and another to create add only the graphical view, one will overwrite the record fleet.fleet_vehicle_action created by the other if done as mentioned below:

fleet_vehicle_pivot

    <record id="fleet.fleet_vehicle_action" model="ir.actions.act_window">
        <field name="view_mode">kanban,tree,**pivot**,activity,form</field>
    </record>

fleet_vehicle_graph

    <record id="fleet.fleet_vehicle_action" model="ir.actions.act_window">
        <field name="view_mode">kanban,tree,**graph**,activity,form</field>
    </record>

The alternative would be to implement in both modules the following as mentioned below:

    <record id="fleet.fleet_vehicle_action" model="ir.actions.act_window">
        <field name="view_mode">kanban,tree,**pivot**,**graph**,activity,form</field>
    </record>

If someone implements a module that adds the calendar view, they will overwrite it too.

    <record id="fleet.fleet_vehicle_action" model="ir.actions.act_window">
        <field name="view_mode">kanban,tree,**calendar**,activity,form</field>
    </record>

I can create a single module by adding the pivot table and the graph view. Do you have a suggested name for the module if you think it is the most suitable option?

  • fleet_vehicle_additional_view
  • fleet_vehicle_pivot_graph
  • fleet_vehicle_view_extension
  • fleet_usability (generic name)
  • fleet_vehicle_usability (generic name)
  • fleet_vehicle_report (as suggested by you) (I don't know if it would be the most suitable name but that's fine by me)

The calendar view can also be created, but it lacks a relevant date to use since the vehicle data model has only the vehicle acquisition date and it does not seem valid to implement the calendar view.

Anyway, it is a small problem, but a little boring to deal with in my opinion.

Let's go for this one, it it's ok to you: fleet_vehicle_pivot_graph

I don't like additional, extension or usability because they don't actually say anything.
However I see your point, it's a small and boring problem.

Alternatively a you can achieve that level of compatibility with other modules by using both post_init_hook and uninstall_hook to effectively add and remove those types of views without overwritting the previous values.

But either way is ok by me !

@marcelsavegnago marcelsavegnago force-pushed the 12.0-add-fleet_vehicle_additional_view branch from aced5c5 to 65402e2 Compare April 1, 2021 14:53
@marcelsavegnago marcelsavegnago changed the title [12.0][ADD] add fleet_vehicle_additional_view (temporary name) [12.0][ADD] add fleet_vehicle_pivot_graph Apr 1, 2021
@marcelsavegnago
Copy link
Member Author

Let's go for this one, it it's ok to you: fleet_vehicle_pivot_graph

@ivantodorovich done!

Alternatively a you can achieve that level of compatibility with other modules by using both post_init_hook and uninstall_hook to effectively add and remove those types of views without overwritting the previous values.

Ok .. thanks for the explanation. I'm going to study hooks.

@marcelsavegnago
Copy link
Member Author

ping @mamcode @mymage

Copy link
Member

@mymage mymage left a comment

Choose a reason for hiding this comment

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

I tried to follow your observation on the problem of overwriting extension but I'm not so confident with code to say something. Anyway the result for me is ok.

@ivantodorovich
Copy link

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 12.0-ocabot-merge-pr-56-by-ivantodorovich-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Apr 2, 2021
Signed-off-by ivantodorovich
@OCA-git-bot
Copy link
Contributor

It looks like something changed on 12.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 12.0-ocabot-merge-pr-56-by-ivantodorovich-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Apr 2, 2021
Signed-off-by ivantodorovich
@OCA-git-bot
Copy link
Contributor

It looks like something changed on 12.0 in the meantime.
Let me try again (no action is required from you).
Prepared branch 12.0-ocabot-merge-pr-56-by-ivantodorovich-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 0bc5c68 into OCA:12.0 Apr 2, 2021
@OCA-git-bot
Copy link
Contributor

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

@marcelsavegnago marcelsavegnago deleted the 12.0-add-fleet_vehicle_additional_view branch April 30, 2021 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants