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 missing workflow_step #139

Conversation

JerRMartin
Copy link
Contributor

Motivation

Which issue does this fix? N/A

If no issue exists, what is the fix or new feature? Were there any reasons to fix/implement things that are not obvious?

  • adding in workflow_step field as part of System class
    • Documentation here lists workflow_step in the response, but Java SDK has no field to access this data on an initialized System Object.

Checklist

  • Code follows coding conventions held in this repo
  • Automated tests have been added
  • Tests are passing
  • Docs have been updated (if applicable)
  • Temporary settings (e.g. variables used during development and testing) have been reverted to defaults

How to test

If manual testing is required, what are the steps?

Step 1:

Tip: lines 11 and (28-42) can be commented out.

In the Main.java file, edit the DeliveryOptions on line 13 to target a test Kontent Project and to look like this:

DeliveryOptions options = DeliveryOptions.builder()
                .projectId("<YOUR-PROJECT-ID>")
                .retryAttempts(2)
                .waitForLoadingNewContent(true)
                .previewApiKey("<YOUR-PREVIEW-API-KEY>")
                .usePreviewApi(true)
                .build();

Step 2:

Rename client to client2 on line 23, then remove the .getName() from line 25, and reference an item in the test Kontent Project like so:

        client2.getItem("<KONTENT-ITEM-CODENAME>")
                .thenAccept(item ->
                        System.out.println(item.getItem().getSystem()));

Expected Results:

When targeting a test project with an unpublished element you should see a System object printed out displaying the workflow_step as one of the fields like so:

System(id=<SOME-ID>, name=<SOME-NAME>, codename=<SOME-CODENAME>, language=default, type=<SOME-TYPE>, collection=default, sitemapLocations=[], lastModified=<SOME-TIMESTAMP>, workflowStep=draft)

Copy link
Contributor

@Simply007 Simply007 left a comment

Choose a reason for hiding this comment

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

Hello @JerRMartin,

thanks for the PR. The change itself seems fine. But the tests are not fully updated and I would love to have this information also in the readme.

According to the changes, it is conceptually the same as in including collection to the SDK:

So please adjust all JSON mock files, extend the binding test and filtering test and extend readme and the PR should be ready to go!

The workflow_step is only being set for all content items - for rich text components it is not: https://kontent.ai/learn/reference/delivery-api/#section/Content-item-object, but I haven't found any components in testing data. It would be great to extend the SampleContentItem.json's description element with one component without having workflow steps and test if it is correctly being set. I would keep this one optional and we can submit a separate issue if not included in this PR.

Once these are in place, I can publish a snapshot to the maven central.

Testing for `Workflow_step` on ContentItems, but does not exist on RichText Components
Copy link
Contributor

@Simply007 Simply007 left a comment

Choose a reason for hiding this comment

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

Thank you for your edits!

Could you reformat README.md to keep its shape and just leave the Workflow step part addition?

@JerRMartin
Copy link
Contributor Author

Thank you for your edits!

Could you reformat README.md to keep its shape and just leave the Workflow step part addition?

I'm sorry, I don't know that I understood what you meant by this. Would you mind rewording it?

@Simply007
Copy link
Contributor

Thank you for your edits!
Could you reformat README.md to keep its shape and just leave the Workflow step part addition?

I'm sorry, I don't know that I understood what you meant by this. Would you mind rewording it?

Sure, In commit 0567fb2 you adjusted the readme with these changes which are completely correct.

image

But there are other changes that are changing the format of the README, like this one:
image
So I would like to have these other changes reverted.

@JerRMartin
Copy link
Contributor Author

Thank you for your edits!
Could you reformat README.md to keep its shape and just leave the Workflow step part addition?

I'm sorry, I don't know that I understood what you meant by this. Would you mind rewording it?

Sure, In commit 0567fb2 you adjusted the readme with these changes which are completely correct.

image

But there are other changes that are changing the format of the README, like this one: image So I would like to have these other changes reverted.

Sorry about that, it seems like the Markdown editor I used auto formatted the file without me noticing. Good Catch!

Copy link
Contributor

@Simply007 Simply007 left a comment

Choose a reason for hiding this comment

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

Thank you @JerRMartin,

changes look OK. I have adjusted two minor things.

Later today, I will merge the PR and release the snapshot for testing. If everything works OK, I can promote shapshot to a new minor version!

@Simply007 Simply007 merged commit e920e8a into kontent-ai:master Jun 20, 2022
@Simply007
Copy link
Contributor

Hello @JerRMartin,

I have merged the PR and released 4.6.0-SNAPSHOT.

Could you test out, if everything works as expected on your particular project (the guide is here)?

Once confirmed, I can promote the snapshot to the production version.

@JerRMartin
Copy link
Contributor Author

JerRMartin commented Jun 20, 2022

Hello @JerRMartin,

I have merged the PR and released 4.6.0-SNAPSHOT.

Could you test out, if everything works as expected on your particular project (the guide is here)?

Once confirmed, I can promote the snapshot to the production version.

@Simply007 Looks Great! I'd say its ready to be promoted!
Screen Shot 2022-06-20 at 12 47 47 PM

@Simply007
Copy link
Contributor

Version 4.6.0 is out!

https://repo1.maven.org/maven2/com/github/kentico/kontent-delivery/

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.

None yet

2 participants