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

Adding generateTenantToken method to the client #297

Merged
merged 15 commits into from
Mar 7, 2022

Conversation

alallema
Copy link
Contributor

@alallema alallema commented Feb 16, 2022

Tenant tokens

Introduction of the new method generateTenantToken in order to facilitate the generation of the tenant token.

Related to:

src/Delegates/HandlesSystem.php Outdated Show resolved Hide resolved
src/Delegates/HandlesSystem.php Outdated Show resolved Hide resolved
src/Delegates/HandlesSystem.php Outdated Show resolved Hide resolved
src/Delegates/HandlesSystem.php Outdated Show resolved Hide resolved
@alallema alallema force-pushed the generate_token branch 6 times, most recently from 55c0e20 to de9ef9e Compare March 1, 2022 12:35
@alallema alallema marked this pull request as ready for review March 2, 2022 17:19
src/Delegates/HandlesSystem.php Show resolved Hide resolved
tests/Endpoints/TenantTokenTest.php Outdated Show resolved Hide resolved
tests/Endpoints/TenantTokenTest.php Outdated Show resolved Hide resolved
tests/Endpoints/TenantTokenTest.php Show resolved Hide resolved
tests/Endpoints/TenantTokenTest.php Outdated Show resolved Hide resolved
tests/Endpoints/TenantTokenTest.php Show resolved Hide resolved
*
* The $options parameter is an array, and the following keys are accepted:
* - apiKey: The API key parent of the token. If you leave it empty the client API Key will be used.
* - expiresAt: A DateTime when the key will expire. Note that if an expiresAt value is included it should be in UTC time.
Copy link
Member

Choose a reason for hiding this comment

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

What about add some validation about this UTC requirement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are we sure we want to force the user to use the UTC? Rather than putting a note of information?

@alallema alallema requested review from brunoocasali and removed request for curquiza March 3, 2022 09:35
tests/Endpoints/TenantTokenTest.php Show resolved Hide resolved
tests/Endpoints/TenantTokenTest.php Show resolved Hide resolved
$this->assertArrayHasKey('hits', $response->toArray());
$this->assertArrayHasKey('query', $response->toArray());
$this->expectException(ApiException::class);
$response = $tokenClient->index('tenantTokenDuplicate')->search('');
Copy link
Member

Choose a reason for hiding this comment

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

I think I didn't follow why this test is testing 😬

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not mandatory at all I just test if my token had access to the right index depending on the token's searchRules.

@alallema alallema requested a review from brunoocasali March 7, 2022 09:10
src/Endpoints/TenantToken.php Outdated Show resolved Hide resolved
Comment on lines 32 to 33
if (null == $searchRules) {
throw InvalidArgumentException::emptyArgument('search rules');
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I change it for:

        if ((!is_array($searchRules) && !is_object($searchRules)) || null == $searchRules) {
            throw InvalidArgumentException::emptyArgument('search rules');
        }

For checking and the type and the fact that searchRules exist

src/Endpoints/TenantToken.php Show resolved Hide resolved
@alallema alallema requested a review from bidoubiwa March 7, 2022 12:13
Copy link
Member

@brunoocasali brunoocasali left a comment

Choose a reason for hiding this comment

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

🔥

@alallema
Copy link
Contributor Author

alallema commented Mar 7, 2022

bors merge

bors bot added a commit that referenced this pull request Mar 7, 2022
297: Adding generateTenantToken method to the client r=alallema a=alallema

## Tenant tokens

Introduction of the new method `generateTenantToken`  in order to facilitate the generation of the tenant token.

Related to:
- this issue: meilisearch/meilisearch#1991
- this spec: meilisearch/specifications#89


Co-authored-by: alallema <amelie@meilisearch.com>
Co-authored-by: Amélie <alallema@users.noreply.github.com>
@bors
Copy link
Contributor

bors bot commented Mar 7, 2022

Build failed:

@alallema alallema merged commit df48591 into bump-meilisearch-v0.26.0 Mar 7, 2022
@alallema alallema deleted the generate_token branch March 7, 2022 16:11
@alallema alallema added the enhancement New feature or request label Mar 9, 2022
bors bot added a commit that referenced this pull request Mar 14, 2022
292: Changes related to the next Meilisearch release (v0.26.0) r=alallema a=meili-bot

Related to this issue: meilisearch/integration-guides#181

This PR:
- gathers the changes related to the next Meilisearch release (v0.26.0) so that this package is ready when the official release is out.
- should pass the tests against the [latest pre-release of Meilisearch](https://github.com/meilisearch/meilisearch/releases).
- might eventually contain test failures until the Meilisearch v0.26.0 is out.

⚠️ This PR should NOT be merged until the next release of Meilisearch (v0.26.0) is out.

_This PR is auto-generated for the [pre-release week](https://github.com/meilisearch/integration-guides/blob/master/guides/pre-release-week.md) purpose._

Done:
- #296
- #298 
- #297
- #302 

Co-authored-by: meili-bot <74670311+meili-bot@users.noreply.github.com>
Co-authored-by: Amélie <alallema@users.noreply.github.com>
Co-authored-by: bors[bot] <26634292+bors[bot]@users.noreply.github.com>
Co-authored-by: alallema <amelie@meilisearch.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants