Skip to content

Create new Details class and add Details field in Task class#336

Merged
bors[bot] merged 6 commits intomeilisearch:mainfrom
kuruvasatya:AddDetailsFieldToTaskClass
Feb 9, 2022
Merged

Create new Details class and add Details field in Task class#336
bors[bot] merged 6 commits intomeilisearch:mainfrom
kuruvasatya:AddDetailsFieldToTaskClass

Conversation

@kuruvasatya
Copy link
Copy Markdown
Contributor

Pull Request

What does this PR do?

Fixes #331
Created new Details class and added the Details field to Task class

PR checklist

Please check if your PR fulfills the following requirements:

  • [ x ] Does this PR fix an existing issue?
  • [ x ] Have you read the contributing guidelines?
  • [ x ] Have you made sure that the title is accurate and descriptive of the changes?

Thank you so much for contributing to MeiliSearch!

Copy link
Copy Markdown
Contributor

@alallema alallema left a comment

Choose a reason for hiding this comment

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

Thanks, @kuruvasatya for this PR!
The code is great can you just add some tests to check if the details class works well?
In this file src/test/java/com/meilisearch/integration/TasksTest.java there are tests on the Task class you can just add for example in the testClientGetTask:

assertNotNull(task.getDetails());
assertNull(task.getDetails().getPrimaryKey());

@kuruvasatya
Copy link
Copy Markdown
Contributor Author

Hey @alallema , can you tell what I am doing wrong

@alallema
Copy link
Copy Markdown
Contributor

alallema commented Feb 9, 2022

Hey @alallema , can you tell what I am doing wrong

Hi @kuruvasatya,
Yes, you try to test the primaryKey in details fields for all tasks and some are null but some are equal to a value. That's why it's failed don't bother you, you can remove this lines 63-65:

            if (task.getType().equals("indexCreation")) {
                assertNull(task.getDetails().getPrimaryKey());
            }

You rest of your tests is enough! Good job 🔥

@kuruvasatya kuruvasatya requested a review from alallema February 9, 2022 13:04
Copy link
Copy Markdown
Contributor

@alallema alallema left a comment

Choose a reason for hiding this comment

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

LGTM!
Thank for contributing to Meilisearch ❤️

@alallema
Copy link
Copy Markdown
Contributor

alallema commented Feb 9, 2022

bors merge

@bors
Copy link
Copy Markdown
Contributor

bors bot commented Feb 9, 2022

Build succeeded:

@bors bors bot merged commit a5df0a4 into meilisearch:main Feb 9, 2022
@alallema alallema added the enhancement New feature or request label Mar 14, 2022
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.

Adding details field to Task class

2 participants