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

[15.0][MIG] website_event_filter_city: Migration to 15.0 #309

Merged
merged 17 commits into from
Apr 5, 2023

Conversation

chienandalu
Copy link
Member

@chienandalu chienandalu commented Apr 3, 2023

  • Refactor to avoid monkeypatching
  • Templates adaptations
  • Tests adaptations
  • Standard migration stuff

cc @Tecnativa TT41028

please review @pedrobaeza @CarlosRoca13

yajo and others added 16 commits March 14, 2023 17:01
You can use selectors above the event list in the website, instead of the left column that there is right now.

Store user customizations in separate template.

Acknowledge that you cannot hide only the date filter.

Remove search button; search on selection change.

Fix bug where date did not match what was being filtered.

[FIX][8.0][website_event_filter_selector] Do not die without types or countries

This basically fixes OCA#44 and a similar bug detected that would happen when no countries were found.

It just adds a `t-if` to avoid possible failures, [borrowing the idea from core `website_event` module](https://github.com/odoo/odoo/blob/8.0/addons/website_event/views/website_event.xml#L167).

[8.0][IMP][website_event_filter_selector] Add city filter for events.

Since this includes new logic, it adds also a template for left-column filters.

With this patch, you can now filter events depending on their cities, not only on their countries.

This also includes some modifications that will make filters occupy always a full bootstrap row, no matter if there are 1, 2, 3 or 4 filters, to avoid ugly layouts.
[IMP] website_event_filter_selector: Spanish translation

[IMP] website_event_filter_selector: German translation

[FIX] website_event_filter_selector: Fix date selection

[FIX] website_event_filter_selector: Fix city count

[FIX] website_event_filter_selector: Don't lose context with new obj
Relicensed to LGPL, new JS API.

Add test. Fix errors found with it.

Go 100% coverage. Support online events.

[FIX][website_event_filter_selector] Fix Travis build

Builds failed on 2017-03-31 because the tour was expecting to see USA event starting in this month, but next day was another month.

Well, in short, this fix makes the tour work on any day.
Includes update to use new tours api.

Patch event counts when a city is searched.
[IMP] Use new README approach

[IMP] Removed the use of @Class

[FIX] Column classes

[UPD] README.rst

[UPD] README.rst
- Use own demo data instead of upstream.
- Fix some JS code that wasn't working as expected.
- Remove dependency on left column; just test own filters widget.

[UPD] README.rst

[UPD] README.rst
With BS3, all these complex computations were needed to always display all filters at full width.

With BS4 it's not needed anymore.

@Tecnativa TT17632

website_event_filter_selector 12.0.1.1.0
Use `custom-select` in `<select>` elements instead of just `form-control`. This way BS4 can style the dropdowns more freely and they look more consistent with the rest of the site.

I also delete a view that is no longer used.

TT17632

website_event_filter_selector 12.0.1.2.0
To reproduce the problem:

1. Create an event on website 2.
2. Do not publish it.
3. Go to website 1.

If you were logged in, that event still appeared as unpublished. It shouldn't happen because it belongs to other website, no matter whether it is published or not.

@Tecnativa TT29748
- Refactor to try to reuse as much from the core as possible. Anyway, as
there's no external method to compose the domain we still have to redo
some things in order to add our city filter stuff.
- Consider the new name search term.
- Fix test: it didn't expect online events when filtering by country,
but Odoo core allows it explicitly.

TT34958
@chienandalu chienandalu changed the title [MIG] website_event_filter_city: Migration to 15.0 [15.0][MIG] website_event_filter_city: Migration to 15.0 Apr 3, 2023
@chienandalu chienandalu force-pushed the 15.0-mig-website_event_filter_city branch from ecac2f6 to b708c2b Compare April 3, 2023 14:53
Copy link
Contributor

@CarlosRoca13 CarlosRoca13 left a comment

Choose a reason for hiding this comment

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

Code and functional review

Please review the tests

@chienandalu chienandalu force-pushed the 15.0-mig-website_event_filter_city branch 2 times, most recently from f745e6f to 0909dea Compare April 4, 2023 15:33
@chienandalu
Copy link
Member Author

Tests fixed

@chienandalu chienandalu force-pushed the 15.0-mig-website_event_filter_city branch from 0909dea to 004c70c Compare April 4, 2023 16:10
@pedrobaeza
Copy link
Member

/ocabot migration website_event_filter_city

@OCA-git-bot OCA-git-bot added this to the 15.0 milestone Apr 4, 2023
@OCA-git-bot OCA-git-bot mentioned this pull request Apr 4, 2023
16 tasks
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 15.0-ocabot-merge-pr-309-by-pedrobaeza-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Apr 4, 2023
Signed-off-by pedrobaeza
@OCA-git-bot
Copy link
Contributor

@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 15.0-ocabot-merge-pr-309-by-pedrobaeza-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

1 similar comment
@OCA-git-bot
Copy link
Contributor

@pedrobaeza your merge command was aborted due to failed check(s), which you can inspect on this commit of 15.0-ocabot-merge-pr-309-by-pedrobaeza-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@chienandalu
Copy link
Member Author

The merge failed although for another module's tests

@pedrobaeza
Copy link
Member

I think it was the "midnight case".

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 15.0-ocabot-merge-pr-309-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit c86e344 into OCA:15.0 Apr 5, 2023
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.

9 participants