-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
feat: add initial workflow status in config #6469
base: main
Are you sure you want to change the base?
feat: add initial workflow status in config #6469
Conversation
1be43b9
to
049c87a
Compare
I don't really know where to add the expected test, if you can guide me :) |
.github/PULL_REQUEST_TEMPLATE.md
Outdated
@@ -29,7 +29,7 @@ Example: The exact commands you ran and their output, screenshots / videos if th | |||
|
|||
Please add a `x` inside each checkbox: | |||
|
|||
- [ ] I have read the [contribution guidelines](../CONTRIBUTING.md). | |||
- [ ] I have read the [contribution guidelines](https://github.com/netlify/netlify-cms/blob/master/CONTRIBUTING.md). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should have gone to a different PR but I'm going to accept it since its scope is small.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very neat editing.
As for testing, I haven't tried it out but it might be about checking if the unpublished entry object has the default_workflow_status
property with the value that a user sets in the config.
I think it could go something like this:
- create a placeholder
config
object which contains thedefault_workflow_status
property - create a new backend instance, each for all the supported backends.
- create a
state
object which contains theconfig
object:state = { config, ... }
- call the
unpublishedEntry
method of theBackend
class, passing to it thestate
object. - check if the result contains a
default_workflow_status
property whose value is whatever set in theconfig
object
What do you think?
Hello @bytrangle, thanks for your review. I tried when I start the backend, and everything work right. I tried here : And here :
Can you give me a little help please? |
On further investigation, I think the test should be in I got the idea from the test "should set publish_mode from config". I've tried create a barebone Then I called For example, Then I check if You'll also need to modify the Does that make sense? |
@bytrangle Oh nice ! It's really clear, I will do that :) Just for your last sentence, you really think it's useful with the following code ? With this line if there is no default_workflow_status it take the first status (draft one) thanks again for your help |
@bytrangle I just add a fixup commit with some change and the test, tell me if this sounds good to you :) |
4798191
to
78cc7e0
Compare
ac67514
to
005cf94
Compare
@krider2010 Look forward to your review. |
Just a heads up I'm working on being added to the maintainer group for review reasons. |
@bytrangle There is no other reviewer who can watch this pr? :) |
@bytrangle Small up please, this feature would allow us to save a lot of time in our processes 🙈 |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
It is still relevant today 🙈 |
waiting for this to happen |
@RomainOliva are you still interested in moving this forward? |
@martinjagodic Yes, it can be interesting to merge it 😇 |
Great! Can you solve the merge conflicts? They mostly emerged because of the package rename. |
08255b6
to
cb2ee3d
Compare
|
Name | Link |
---|---|
🔨 Latest commit | cb2ee3d |
✅ Deploy Preview for decap-www ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@martinjagodic sorry for the delay, it's all good the PR is rebased :) |
Summary
closes #1306
For our case we now use the editional workflow, we use it to preview our changes, but sometimes we just want to publish directly.
And this is not possible, we have to save, change status to ready and publish.
I would have preferred to do this issue #4054 but it seemed more complex for a first issue on the project :)
The goal here is to add the initial status in the configuration, then in our cas we can put
PENDING_PUBLISH
status by default and then publish directly.Test plan
Checklist
Please add a
x
inside each checkbox:yarn format
.yarn test
.