[GH-ISSUE #269] Add GET endpoint to add bookmarks to use with bookmarklet and other methods #193

Open
opened 2026-02-25 23:33:40 +03:00 by kerem · 4 comments
Owner

Originally created by @deanishe on GitHub (Aug 20, 2020).
Original GitHub issue: https://github.com/go-shiori/shiori/issues/269

Add GET endpoint for submitting bookmarks via bookmarklet, Web Share API, other integrations.

Acceptance criteria

  • GET /bookmarklet: Only for authenticated owners. Receive url via get parameter: /bookmarklet?url=(urlencoded url).
  • Endpoint should be properly tested.
  • Endpoint require the user to be an authenticated owner. (Authentication methods?)
Originally created by @deanishe on GitHub (Aug 20, 2020). Original GitHub issue: https://github.com/go-shiori/shiori/issues/269 Add GET endpoint for submitting bookmarks via bookmarklet, Web Share API, other integrations. ## Acceptance criteria - `GET /bookmarklet`: Only for authenticated owners. Receive url via get parameter: `/bookmarklet?url=(urlencoded url)`. - Endpoint should be properly tested. - Endpoint require the user to be an authenticated owner. (Authentication methods?)
Author
Owner

@akavel commented on GitHub (May 30, 2024):

Hi! I would be interested to try if I could implement this; would you have any pointers for how to approach that for a person not familiar with the codebase? also, how the authentication should work in this case? also, any specifics on how to & how not to implement this? TIA! @fmartingr ?

I would like to try do it in coordination with the amazing maintainers of this project, the way you'd like it to be done, so that you hopefully get more return from this than trouble 😂 I am happy for discussions and back and forth on the PR once it is ready. I'm a maintainer myself, so I know how the things look from the other side. I'd just be grateful for some guidelines for start - for one thing, to begin in roughly good for you direction instead of wasting time on random ideas that would show up be completely against your vision; for the second thing, you know the codebase way better than me so you can easily point to me some initial appropriate places; for the third thing, it could give us a tiny bit better chance at having this maybe actually merged in the future 😂 Anyway, if that's too much effort for you at this moment, I'm totally fine with that too - just please let me know so I can try to dive head-first and just discuss later on a PR (if I even get there). However little or much you can tell me, I'll be grateful - just I'd love rather faster than later if possible 😅❤️ but still ok if not, I will understand. Cheers!

<!-- gh-comment-id:2139064091 --> @akavel commented on GitHub (May 30, 2024): Hi! I would be interested to try if I could implement this; would you have any pointers for how to approach that for a person not familiar with the codebase? also, how the authentication should work in this case? also, any specifics on how to & how not to implement this? TIA! @fmartingr ? I would like to try do it in coordination with the amazing maintainers of this project, the way you'd like it to be done, so that you hopefully get more return from this than trouble 😂 I am happy for discussions and back and forth on the PR once it is ready. I'm a maintainer myself, so I know how the things look from the other side. I'd just be grateful for some guidelines for start - for one thing, to begin in roughly good for you direction instead of wasting time on random ideas that would show up be completely against your vision; for the second thing, you know the codebase way better than me so you can easily point to me some initial appropriate places; for the third thing, it could give us a tiny bit better chance at having this maybe actually merged in the future 😂 Anyway, if that's too much effort for you at this moment, I'm totally fine with that too - just please let me know so I can try to dive head-first and just discuss later on a PR (if I even get there). However little or much you can tell me, I'll be grateful - just I'd love rather faster than later if possible 😅❤️ but still ok if not, I will understand. Cheers!
Author
Owner

@fmartingr commented on GitHub (May 31, 2024):

Hey @akavel, thank you for volunteering! Some details as how I saw this happening:

  • A new route (like /bookmarks/add or /bookmarks/bookmarklet) living under internal/http/routes/bookmark.go
  • A new method under the BookmarkDomain in charge of adding bookmarks. Right now that can be just a passthrough to call the database, but in the future we would want to differentiate database and publicly exposed objects and this will be the layer in charge of that.
  • The current method to save bookmarks is SaveBookmarks. You can access it from the domain under d.deps.Database.SaveBookmarks.
  • Testing, we need to increase that coverage rating little by little

This is briefly what I've been thinking about this. Let me know if you have further questions or if you think something would be done in a different way things are obviously up for discussion.

Thanks again!

<!-- gh-comment-id:2141402838 --> @fmartingr commented on GitHub (May 31, 2024): Hey @akavel, thank you for volunteering! Some details as how I saw this happening: - A new route (like `/bookmarks/add` or `/bookmarks/bookmarklet`) living under [`internal/http/routes/bookmark.go`](https://github.com/go-shiori/shiori/blob/master/internal/http/routes/bookmark.go) - A new method under the [`BookmarkDomain`](https://github.com/go-shiori/shiori/blob/master/internal/domains/bookmarks.go#L11) in charge of adding bookmarks. Right now that can be just a passthrough to call the database, but in the future we would want to differentiate database and publicly exposed objects and this will be the layer in charge of that. - The current method to save bookmarks is [`SaveBookmarks`](https://github.com/go-shiori/shiori/blob/master/internal/database/database.go#L80). You can access it from the domain under `d.deps.Database.SaveBookmarks`. - Testing, we need to increase that coverage rating little by little This is briefly what I've been thinking about this. Let me know if you have further questions or if you think something would be done in a different way things are obviously up for discussion. Thanks again!
Author
Owner

@akavel commented on GitHub (May 31, 2024):

Thanks @fmartingr for a quick reply! ❤️❤️

One thing that is not super clear to me, both from implementation, but also esp. usage and maybe testing perspective, is how auth should work here? In particular:

  • is there some existing mechanism that will automagically work for a new endpoint, or maybe it requires opt-in? this I presume might become obvious in codebase, so here asking just in case; but still:
  • I saw some mention of jwt somewhere, is that used for auth? is it via cookies? esp. from usage/user-story perspective, for a bookmarklet user, I wonder would they have to re-login very often? or maybe even store user+pwd locally and just send everytime to pre-login and get jwt token every time? I'm actually interested in trying to build a bookmarklet for myself afterwards 😂 so I want to know how I can use it for that, i.e. it's core purpose 😄
  • you mention tests with some concern, so I feel a need to ask now, is there some existing testing infra I can plug into and see for inspiration? esp. related to auth? or maybe I should build some part of the infra? (should be ok for me to do anyway, just want to clarify - I presume might also be clear in the code, but asking primarily because of the perceived concern in your tone 😅)
<!-- gh-comment-id:2142649013 --> @akavel commented on GitHub (May 31, 2024): Thanks @fmartingr for a quick reply! ❤️❤️ One thing that is not super clear to me, both from implementation, but also esp. usage and maybe testing perspective, is how auth should work here? In particular: - is there some existing mechanism that will automagically work for a new endpoint, or maybe it requires opt-in? this I presume might become obvious in codebase, so here asking just in case; but still: - I saw some mention of jwt somewhere, is that used for auth? is it via cookies? esp. from usage/user-story perspective, for a bookmarklet user, I wonder would they have to re-login very often? or maybe even store user+pwd locally and just send everytime to pre-login and get jwt token every time? I'm actually interested in trying to build a bookmarklet for myself afterwards 😂 so I want to know how I can use it for that, i.e. it's core purpose 😄 - you mention tests with some concern, so I feel a need to ask now, is there some existing testing infra I can plug into and see for inspiration? esp. related to auth? or maybe I should build some part of the infra? (should be ok for me to do anyway, just want to clarify - I presume might also be clear in the code, but asking primarily because of the perceived concern in your tone 😅)
Author
Owner

@fmartingr commented on GitHub (May 31, 2024):

  • is there some existing mechanism that will automagically work for a new endpoint, or maybe it requires opt-in? this I presume might become obvious in codebase, so here asking just in case; but still:

You only need to add the handler in the bookmark.go file.

  • I saw some mention of jwt somewhere, is that used for auth? is it via cookies? esp. from usage/user-story perspective, for a bookmarklet user, I wonder would they have to re-login very often? or maybe even store user+pwd locally and just send everytime to pre-login and get jwt token every time? I'm actually interested in trying to build a bookmarklet for myself afterwards 😂 so I want to know how I can use it for that, i.e. it's core purpose 😄

Since the main purpose of this endpoint would be using it from a bookmarklet I'd say cookie auth is enough. Authentication is available automatically in all routes via middlewares for both JWT and Cookies.

  • you mention tests with some concern, so I feel a need to ask now, is there some existing testing infra I can plug into and see for inspiration? esp. related to auth? or maybe I should build some part of the infra? (should be ok for me to do anyway, just want to clarify - I presume might also be clear in the code, but asking primarily because of the perceived concern in your tone 😅)

My concern is mostly to try and test all new additions to the codebase because most of the code is currently not covered and since we are undergoing some refactors I expect basic test of the new features/refactors being worked on. Regarding how to test, you can test the handler directly, keep an eye on other tests to check how we did it. We have a bunch of helpers under testutil to ease out asserts done to HTTP handlers, I believe is mainly focused for the API but it can be used/adapted for other handlers.

<!-- gh-comment-id:2142821224 --> @fmartingr commented on GitHub (May 31, 2024): > * is there some existing mechanism that will automagically work for a new endpoint, or maybe it requires opt-in? this I presume might become obvious in codebase, so here asking just in case; but still: You only need to add the handler in the [bookmark.go](https://github.com/go-shiori/shiori/blob/master/internal/http/routes/bookmark.go) file. > * I saw some mention of jwt somewhere, is that used for auth? is it via cookies? esp. from usage/user-story perspective, for a bookmarklet user, I wonder would they have to re-login very often? or maybe even store user+pwd locally and just send everytime to pre-login and get jwt token every time? I'm actually interested in trying to build a bookmarklet for myself afterwards 😂 so I want to know how I can use it for that, i.e. it's core purpose 😄 Since the main purpose of this endpoint would be using it from a bookmarklet I'd say cookie auth is enough. Authentication is available automatically in all routes via [middlewares](https://github.com/go-shiori/shiori/blob/master/internal/http/middleware/auth.go) for both JWT and Cookies. > * you mention tests with some concern, so I feel a need to ask now, is there some existing testing infra I can plug into and see for inspiration? esp. related to auth? or maybe I should build some part of the infra? (should be ok for me to do anyway, just want to clarify - I presume might also be clear in the code, but asking primarily because of the perceived concern in your tone 😅) My concern is mostly to try and test all new additions to the codebase because most of the code is currently not covered and since we are undergoing some refactors I expect basic test of the new features/refactors being worked on. Regarding how to test, you can test the handler directly, keep an eye on other tests to check how we did it. We have a bunch of helpers under [testutil](https://github.com/go-shiori/shiori/tree/master/internal/testutil) to ease out asserts done to HTTP handlers, I believe is mainly focused for the API but it can be used/adapted for other handlers.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
starred/shiori#193
No description provided.