-
-
Notifications
You must be signed in to change notification settings - Fork 161
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
[13.0][mig] website_event_snippet_calendar #213
[13.0][mig] website_event_snippet_calendar #213
Conversation
@yajo snippet fixed and working |
added some improvements for the calendar (calendar view and event list) to take care of the filters applied (eg: event category) |
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.
Tested on runbot and it doesn't work:
Peek.12-05-2021.07-50.mp4
Traceback in debug assets mode:
Error:
Traceback:
Error: $(...)[0] is undefined
load_events@http://3482153-213-1b3e23.runbot1-3.odoo-community.org/website_event_snippet_calendar/static/src/js/animation.js:128:25
start@http://3482153-213-1b3e23.runbot1-3.odoo-community.org/website_event_snippet_calendar/static/src/js/animation.js:60:18
OdooClass.extend/</prototype[name]</<@http://3482153-213-1b3e23.runbot1-3.odoo-community.org/web/static/src/js/core/class.js:90:38
attachTo/<@http://3482153-213-1b3e23.runbot1-3.odoo-community.org/web/static/src/js/core/widget.js:207:25
Besides, you have to fix that security hole explained below.
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.
Minor change
c5ed2e8
to
e670dfa
Compare
99a11f4
to
b1dc256
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.
Looks good.
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.
Code looks good, but unfortunately, it still does not work:
Peek.20-05-2021.09-09.mp4
@yajo can you please give me the url of the runbot you are using? I managed to make it work on runbot. Ah BTW, the widget must be placed on the event page ... (that's my usecase at least). |
@yajo seems ok now |
315279e
to
84cb798
Compare
84cb798
to
e5c58cf
Compare
…list (OCA#101) Allows the user to display a calendar combined with a list of events. Includes controllers for such information to be bundled into any page through an `<iframe>`.
Before this patch, the user was getting the Public User's TZ on the calendar view. After merged, only the event's timzeone will be used to display dates. It is, however, affected by odoo/odoo#27179, which means that there is a possible mismatch between the date used to search and the one used to display, most likely to be hit when events have very different timzeones. Added to Known Issues.
Co-Authored-By: Jairo Llopis <Yajo@users.noreply.github.com>
Pretty obvious things missing from BS4 migration. @Tecnativa TT17632
e5c58cf
to
104764e
Compare
104764e
to
db4f817
Compare
Hey @damdam-s looks good. Do you think could it be posible to add a filter by event type? |
<<<<<<< HEAD | ||
name = fields.Char(string="Reason", required=True, translate=True) | ||
======= | ||
name = fields.Char("Reason", required=True, translate=True) | ||
>>>>>>> [IMP] : black, isort, prettier |
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.
Please fix the conflicts instead of committing this.
@@ -54,12 +55,23 @@ def events_for_day(self, day=None, limit=None): | |||
:param limit int: | |||
How many results to return. | |||
""" | |||
|
|||
searches = parse_qsl(searches[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.
This is error-prone. If you do None[1:]
you'll get an error.
BTW I think it would be better if the JS client calls this argument directly with an array of results. Passing in a query string as a JSON argument doesn't seem very ergonomic.
for search in searches: | ||
if search[0] == "type": | ||
try: | ||
value = int(search[1]) | ||
except ValueError: | ||
value = 0 | ||
if value: | ||
domain.append(("event_type_id", "=", int(search[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.
If all you're using searches
for is for filtering by type, it would be better if the parameter were called type
directly and got an int
or List[int]
.
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
/ocabot migraton website_event_snippet_calendar |
Hi @rafaelbn. Your command failed:
Ocabot commands
More information
|
/ocabot migration website_event_snippet_calendar |
There's no issue in this repo with the title 'Migration to version 13.0' and the milestone 13.0, so not possible to add the comment. |
Hello @damdam-s , are you going to attend the review is pending? Hello @Camille0907 are you going to review this PR? |
/ocabot migration website_event_snippet_calendar |
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
superseedes #191