[ADD][15.0] base_ical: Module to expose iCalendar over URL #286
Conversation
fkantelberg
force-pushed
the
15.0-base_ical
branch
from
42996f8 to
e2a132a
Compare
base_ical/controllers/main.py
Outdated
| ) | ||
| def get_ical(self, calendar_id, access_token=None): | ||
| if not access_token: | ||
| raise request.not_found() |
base_ical/controllers/main.py
Outdated
| key=access_token, | ||
| ) | ||
| if not user_id: | ||
| raise request.not_found() |
base_ical/models/base_ical_token.py
Outdated
| from odoo import fields, models | ||
|
|
||
|
|
||
| class BaseIcalToken(models.Model): |
base_ical/controllers/main.py
Outdated
| calendar = ( | ||
| http.request.env["base.ical"] | ||
| .sudo() | ||
| .search([("id", "=", calendar_id)]) |
There was a problem hiding this comment.
Replace search(...) with browse(calendar_id) and relevant exception handling?
There was a problem hiding this comment.
There is a big difference. browse doesn't check against existing (you would have to use exists afterwards which also results in a query. Also search would automatically hide inactive ones. I favor search here.
base_ical/controllers/main.py
Outdated
|
|
||
| calendar = ( | ||
| http.request.env["base.ical"] | ||
| .sudo() |
There was a problem hiding this comment.
Given that users have read access to base.ical model per the security csv, replace sudo() with with_user(user_id) from below?
| if not calendar: | ||
| return request.not_found() | ||
|
|
||
| records = calendar._get_items() |
There was a problem hiding this comment.
My suggestion above means that this call to _get_items() is no longer in sudo() mode and could return fewer results. I'm open to an argument on this.
There was a problem hiding this comment.
Before is wasn't su too. because as @hbrunn mention with_user sets the su flag to false per default. Either way I guess that without sudo would be correct because you don't want to allow access to stuff a user can't see. If you link it with a nextcloud you should do it with an user with proper rights.
There was a problem hiding this comment.
I agree it should be as it is - .with_user but no sudo
| "version": "15.0.1.0.0", | ||
| "development_status": "Alpha", | ||
| "category": "Tools", | ||
| "website": "https://github.com/OCA/server-backend", |
There was a problem hiding this comment.
Should this land in https://github.com/OCA/calendar instead?
There was a problem hiding this comment.
Not sure how we want to sort it. You can publish leaves => holidays. You can publish activities as TODO => social(?). You can publish a calendar => calendar. You can also publish attendance => attendance. server-backend might fit as general place.
You will probably use it in your calendar application but I'm not sure if this is the correct criteria.
fkantelberg
force-pushed
the
15.0-base_ical
branch
from
e2a132a to
d7f8964
Compare
| if not calendar: | ||
| return request.not_found() | ||
|
|
||
| records = calendar._get_items() |
There was a problem hiding this comment.
I agree it should be as it is - .with_user but no sudo
|
This PR has the |
|
/ocabot merge nobump |
|
This PR looks fantastic, let's merge it! |
|
Congratulations, your PR was merged at ad01df9. Thanks a lot for contributing to OCA. ❤️ |
Syncing from upstream OCA/server-backend (16.0)
This PR continues #231 and has implemented the following:
_get_ics_fileto generate events)