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

Add new module 'web_listview_date_range_bar'. #117

Closed

Conversation

codingforfun
Copy link

Adds a new view_mode for list views that adds a date range selection toolbar on top of your list view.

verkaufscharts shop - openerp - chromium_007-4

Adds a new view_mode for list views that adds a date range selection toolbar on top of your list view.
@oca-clabot
Copy link

Hey @codingforfun, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/website.cla
Here is a list of the users:

  • Peter Hahn (no github login found)

Appreciation of efforts,
OCA CLAbot

@legalsylvain
Copy link
Contributor

Hi @codingforfun,
Thanks a lot for your PR.
I did'nt found you in OCA database. Could you consider sign the CLA and send it ?
More information here : https://odoo-community.org/page/cla

Regards.

@OSguard
Copy link

OSguard commented Apr 16, 2015

i can confirm that @codingforfun is a contractor to initOS and we have all rights of his work, so this is cover to our company cla

@legalsylvain
Copy link
Contributor

Yes. Thanks @OSguard.
But IFAIK, @codingforfun must sign an individual CLA. @gurneyalex, Can you confirm that point ?
Kind regards.

@OSguard
Copy link

OSguard commented Apr 16, 2015

I discuss this a couple time and until now the result was that the company CLA is enough (and makes our process more easy).

@gurneyalex
Copy link
Member

@legalsylvain @OSguard current ruling is that ECLA is enough for now for employees, although getting them to sign the CLA is cleaner from a legal pov. The main issue is that people may change companies without the OCA being notified, or issues with people working on their free time and who may not appreciate their contributions being affected by their bosses' CLA.

In this specific case, @codingforfun is a contractor, and the situation is different: I cannot link the record in the database to initOS because AFAIK he could be working for another partner in the future and we don't want this to be related to initOS.

I see 2 options:

  • mention in the commit message that the commit is covered by initOS ECLA
  • have Peter send us a signed ICLA

Bonus question: is ntdorova a contractor or a initOS employee ?

@gurneyalex
Copy link
Member

travis red -> needs fixing

@OSguard
Copy link

OSguard commented Apr 16, 2015

the pull request is from our initOS group. This was my suggestion: company repository + company CLA should be to be separate from changing company or private contributing

bonus answer: ntdorova is a employee

@legalsylvain
Copy link
Contributor

the pull request is from our initOS group...

@OSguard : No I'm not agree with that point. Other people out of your company can commit into your repository. (You can accept PR), and it happens often. Other possibility, you can fork another repository improve it, and propose the improved module into OCA.
The check must be by commiter, except if in the commit, there is the mention of your company.

@OSguard
Copy link

OSguard commented Apr 16, 2015

i mean:
committer assigned to $company + $company repository + $company CLA

'category': '',
'summary': 'Add date range selector bar widget above list view rows.',
'description': """
Date range selelction toolbar for list view widget.
Copy link
Member

Choose a reason for hiding this comment

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

s/selelction/selection

@pedrobaeza
Copy link
Member

Thanks for the contribution. Indeed is a needed feature, but the implementation is a bit akward, needing to change action and overriding search, not allowing to use the date range in more than one field, and so on, so I prefer to go to a more generic module, that allows to search for example in the advanced search by a range when the field is a date.

BTW, I let others give their opinion about this.

@codingforfun
Copy link
Author

@pedrobaeza Thanks for your hints. Maybe you can clarify a bit for me and point me in the right direction.

needing to change action and overriding search

Actually I thought of this as a feature, because i didn't want to tie this to a hard coded field name. Do you have any other idea? I could think of adding the target field name as an optional attribute to the <tree> tag, but my first try to do so didn't worked out, so maybe someone has a hint on this.

needing to change action

This one I don't understand. How should I select using the widget or not once it is installed without specifying the view type in an action? Can you clarify please what should be changed here?

not allowing to use the date range in more than one field

What do you mean exactly? Currently you can filter on everything you like in your overridden version of the search function (it's super generic++ ;-). I don't really understand what you mean by more than one field.

@gurneyalex
Copy link
Member

discussion among the board concluded that this contribution is OK cla-wise

@codingforfun
Copy link
Author

What's the final conclusion here? Any suggestions how this could be improved?
I would like to see this merged to went over to my other pull request which is probably more interesting and depends on this one.
We could just say this is OK as a first version for the 7.0 branch.

AdilHoumadi pushed a commit to taktik/web that referenced this pull request May 4, 2015
[IMP] : improve the way we look for data, taking into account @pedrobaeza remarks
[IMP] : add a selector to select the field in which we gonna filter.
More details : @see : OCA#117
@legalsylvain
Copy link
Contributor

Hi ! @codingforfun.
I just tested your module. It's great.
Some remarks :

  • in your code, you use date_end all the time but sometime "date_start" and sometime "date_from". Could you use always the same word ?

I wrote a little test module.

    <record id="action_test" model="ir.actions.act_window">
        <field name="name">test</field>
        <field name="res_model">sale.order</field>
        <field name="view_type">form</field>
        <field name="view_mode">listview_date_range_bar</field>
        <field name="target">current</field>
    </record>

    <menuitem name="test" id="menu_test"
              parent="base.menu_sales" action="action_test"
              sequence="40"/>

    def search(self, cr, user, args, offset=0, limit=None, order=None, context=None, count=False):
        date_from = context and context.get('list_date_range_bar_start')
        date_end = context and context.get('list_date_range_bar_end')
        if date_from:
            args.append(('date_order','>=', date_from))
        if date_end:
            args.append(('date_order','<', date_end))
        return super(SaleOrder, self).search(cr, user, args, offset=offset, limit=limit, order=order,
               context=context, count=count)
  • With that code, On the tree view, I can not click to access to the form view. Did you have the same problem ?
  • I guess that the python code will be always the same. (adding "<=" and "<" filters in current args for a specific field. What about to add an option "target_date_field" in the action, to avoid to overload search function each time ?

Except that points, very good and usefull work. Thanks a lot !

@codingforfun
Copy link
Author

@legalsylvain

  • With that code, On the tree view, I can not click to access to the form view. Did you have the same problem ?

You need to add to the action all the views you would like to have e.g.

    <record id="action_test" model="ir.actions.act_window">
        <field name="name">test</field>
        <field name="res_model">sale.order</field>
        <field name="view_type">form</field>
        <field name="view_mode">listview_date_range_bar,form</field>
        <field name="target">current</field>
    </record>

This way you can switch to the form view. However the customized tree view is missing an icon/button and it's impossible to switch back. This is still an open issue.

I'll look at this and your other points asap.
Thanks for your feedback!

@legalsylvain
Copy link
Contributor

Hi @codingforfun.
thanks for your previews answer. I tested again and it's work, except the bug to switch back to the previous that is quiet annoying. Don't you think ?

About what @pedrobaeza was saying about implementation :

I was thinking about such use :

    <record id="sale.action_quotations" model="ir.actions.act_window">
        <field name="date_range_field_id" ref="sale.field_sale_order_date_order"/>
    </record>

If previous code is used :
-> the tree view is displayed with your extra tools (date range bar);
-> no "switch back" bug (the bug I mentionned) ;
-> no need to overload the search function each time (the problem that @pedrobaeza said);

But it could be an heavy refactoring of your code.

What do you think ?

@luc-demeyer
Copy link

Based upon the screenshot (I haven't tested this module) :

this seems a functional duplicate of #227 (which I have tested intensively in the meanwhile and this one works ok).
I think it's better that we have one OCA module offering the date range on a tree view.

@legalsylvain
Copy link
Contributor

Hi @luc-demeyer ! Thanks for your review.
in fact #227 ( 8.0 / web_tree_date_search) seems to do the same things as this PR 7.0 / web_listview_date_range_bar.

But 'web_tree_date_search' doesn't exist in 7.0 serie and does'nt seem to be backported in 7.0.
So we can imagin the following scenario :
1/ backport web_tree_date_search and close the current PR ;
2/ accept this PR and just mention in this module that it has been superseded in V8.

@codingforfun what do you think ?

regards.

@OSguard
Copy link

OSguard commented Nov 25, 2015

So we have two diffrent version #132 is also related!

@pedrobaeza
Copy link
Member

About this PR, anyone interested? I personally preferred something that doesn't any change in views and that is embedded in the advanced search or at least an approach like the one in https://github.com/OCA/web/tree/8.0/web_search_datetime_completion

BT-fgarbely pushed a commit to BT-fgarbely/web that referenced this pull request Apr 3, 2018
Syncing from upstream OCA/web (10.0)
@pedrobaeza
Copy link
Member

No answer, so I close it. Feel free to reopen if needed.

@pedrobaeza pedrobaeza closed this May 4, 2018
vrenaville pushed a commit to camptocamp/web that referenced this pull request Jul 19, 2018
[FIX] BQL, BDL fields in project analysis report BSMTS-230
davidtranhp pushed a commit to davidtranhp/web that referenced this pull request Feb 7, 2024
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.

None yet

8 participants