-
-
Notifications
You must be signed in to change notification settings - Fork 298
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
added support for all form parameters in bookmarklet endpoint #793
Conversation
@@ -6,7 +6,7 @@ | |||
{{ buku.set_lang() }} | |||
{{ buku.limit_navigation_if_popup() }} | |||
{{ buku.script('bookmark.js') }} | |||
{{ buku.fetch_checkbox() }} | |||
{{ buku.fetch_checkbox(form.fetch.data) }} |
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.
Previously the initial value of this checkbox was hardcoded; now it's based on request parameters.
@@ -209,11 +209,13 @@ def create_form(self, obj=None): | |||
form.url.data = request.args.get('link', form.url.data) | |||
form.title.data = request.args.get('title', form.title.data) | |||
form.description.data = request.args.get('description', form.description.data) | |||
form.tags.data = request.args.get('tags', form.tags.data) | |||
form.fetch.data = request.args.get('fetch', request.data.get('fetch', True)) |
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 invoked in both GET request when fetching the form, and POST request when submitting it.
In either case, if the fetch=
parameter is present it will be used, otherwise falling back to default value.
return form | ||
|
||
def create_model(self, form): | ||
try: | ||
model = types.SimpleNamespace(id=None, url=None, title=None, tags=None, description=None, fetch=True) | ||
model = types.SimpleNamespace(id=None, url=None, title=None, tags=None, description=None, fetch=None) |
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.
…I haven't actually found any difference in behaviour when changing this, but leaving it as True
might be misleading for a reader.
@@ -168,12 +168,14 @@ def test_bookmarklet_view(bukudb, client, exists, uri, tab, args): | |||
(True, '', 'Some description'), | |||
(False, 'Some title', ''), | |||
(False, '', 'Some description'), | |||
(None, 'Some title', ''), | |||
(None, '', 'Some description'), |
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.
Now fetching behaviour is tested for truthy parameter, falsey parameter, and unsupplied value.
'title': (title or _title) if fetch else title, | ||
'description': (desc or _desc) if fetch else desc, | ||
'title': (title or _title) if fetch or fetch is None else title, # defaults to True | ||
'description': (desc or _desc) if fetch or fetch is None else desc, |
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.
Fetch is invoked unless fetch
is both specified and falsey
Thank you! |
Something which I had in my workspace for a week or so…
This doesn't affect current Bukuserver behaviour, but it extends the bookmarklet API to support all fields of the Create Bookmark form; which could be used by custom (e.g. user-made) bookmarklets.