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

New apis for getting one or more collections #15

Merged
merged 9 commits into from
Jul 10, 2023

Conversation

rsjethani
Copy link
Collaborator

@rsjethani rsjethani commented Jul 8, 2023

New sub-clients types for fetching one or more collections.

Update Client creation to take http.Client from user. Also take api key as normal string, no need for making it a secret.Text because we inject it into the request immediately after receiving it. New internal common package which houses types and symbols which are required in both external tests and business logic.

Updated README to reflect above changes and remove not yet existing scenarios.
Also: Updated minimum Go version to 1.19 as we use some new stuff ;)

Closes #2

@rsjethani rsjethani linked an issue Jul 8, 2023 that may be closed by this pull request
2 tasks
@rsjethani rsjethani self-assigned this Jul 8, 2023
Copy link
Collaborator

@StefMa StefMa left a comment

Choose a reason for hiding this comment

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

What do you think about my comment? 🤔

models.go Show resolved Hide resolved
@rsjethani rsjethani changed the title 2 feature apis for getting one or more collections Feature: new apis for getting one or more collections Jul 9, 2023
@rsjethani rsjethani changed the title Feature: new apis for getting one or more collections New apis for getting one or more collections Jul 9, 2023
@rsjethani rsjethani marked this pull request as ready for review July 9, 2023 23:18
@rsjethani rsjethani requested a review from StefMa July 9, 2023 23:18
Copy link
Collaborator

@StefMa StefMa left a comment

Choose a reason for hiding this comment

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

Did not tested it, but looks good to me 👍



// Fetch information about a collection:
col,err := cl.Collections().Get("collection id").Do(context.Background())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
col,err := cl.Collections().Get("collection id").Do(context.Background())
col, err := cl.Collections().Get("collection id").Do(context.Background())

@@ -6,23 +6,27 @@
# Usage

## Client
Getting a single document by its id:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you removed it? 🤔
This is still possible, right?

Get all documents for a collection:
```golang
cl := outline.New()
cl.Documents().GetAll().Collection("collection id").Do(context.Background(), func(d *outline.Document, err error) bool {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you removed this, this is stil possible right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, these were not possible in the first place. They were just added to show how API may look like. I should have used 'github discussions' rather than adding this I guess. But anyways...upcoming PRs will definitely add documents functionality.

ID CollectionID `json:"id"`
}{ID: id}

copy := sl.New()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to get it right, this will "copy" the existing setup of Sling (therefore, copy headers and stuff), correct?

@rsjethani rsjethani merged commit 45b59fd into main Jul 10, 2023
2 checks passed
@rsjethani rsjethani deleted the 2-feature-apis-for-getting-one-or-more-collections branch July 10, 2023 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: APIs for getting one or more collections
2 participants