The latest updates on your projects. Learn more aboutVercel for Git ↗︎

All committers have signed the CLA.

Hi@ankit7201 in order to accept this contribution that looks great we will need the follow things:

• Making it opt-in (you can look at thepopulateCreatorField option for example to avoid making it a Breaking change
• Add unit & api integration tests.

Hi@alexandrebodin

Thank you for the review.

Had a doubt regarding the first point that you mentioned and I was hoping to get some clarity on it.

When you mentioned to make this opt-in, did you mean its visibility will be controlled by a flag in schema (similar to populateCreatorField)

Meaning, it needs to be "private" unless it is enabled in the schema?

Hi@alexandrebodin

Thank you for the review.

Had a doubt regarding the first point that you mentioned and I was hoping to get some clarity on it.

When you mentioned to make this opt-in, did you mean its visibility will be controlled by a flag in schema (similar to populateCreatorField)

Meaning, it needs to be "private" unless it is enabled in the schema?

We need to opt-in adding the field completely not just making it visible. It shouldn't be there by default or it would be a breaking change. It's about wrapping the code where you add the attribute with a condition checking the user has opted in adding it.

We need to opt-in adding the field completely not just making it visible. It shouldn't be there by default or it would be a breaking change. It's about wrapping the code where you add the attribute with a condition checking the user has opted in adding it.

@alexandrebodin Got it.
But, wouldn't this cause problem if a user publishes a content and then enables this field, since in that case, the field would be created but its value won't be the first time this was published or is that the expected behaviour?

What I mean is, since we are not recording the first published at datetime if it is not enabled by default, and some content is published and then we are enabling it, we don't have any value to populate this field.

Or, is the expectation that if this is enabled after the content is published, a user would have to manually update this field to add a value to it.

We need to opt-in adding the field completely not just making it visible. It shouldn't be there by default or it would be a breaking change. It's about wrapping the code where you add the attribute with a condition checking the user has opted in adding it.

@alexandrebodin Got it. But, wouldn't this cause problem if a user publishes a content and then enables this field, since in that case, the field would be created but its value won't be the first time this was published or is that the expected behaviour?

What I mean is, since we are not recording the first published at datetime if it is not enabled by default, and some content is published and then we are enabling it, we don't have any value to populate this field.

Or, is the expectation that if this is enabled after the content is published, a user would have to manually update this field to add a value to it.

Good questions

-> For new content-types created we would generate them with the option enabled so new users would have the field enabled.
-> For existing projects that want this field they would need to add the option. we can create a enable/disable migration. like we have for enabling/disabling Draft & Publish that will set the value when enabling it. you can look at packages/core/core/src/providers/registries.ts and the ../migrations import in there

that will set the value when enabling it

@alexandrebodin What will we set the value to when it is enabled for existing project?

One idea would be to set it same as publishedAt date but that would be inaccurate since a content might be published several times and thereby changing the published date and time when it was first published?

that will set the value when enabling it

@alexandrebodin What will we set the value to when it is enabled for existing project?

One idea would be to set it same as publishedAt date but that would be inaccurate since a content might be published several times and thereby changing the published date and time when it was first published?

Yes it's the best we can do for existing data anyway

@alexandrebodin
And for new apps, since firstPublishedAt date would be true by default, do we need to update the UI as well (similar to draftAndPublish)?

@alexandrebodin And for new apps, since firstPublishedAt date would be true by default, do we need to update the UI as well (similar to draftAndPublish)?

I think we should just make it an option in the json as the goal isn't to easily switch back and forth it's just a way to introduce it in a backward compat way :)

@alexandrebodin

Got it. Thank you for all the help.

Here is the final action plan:

1. Add code to include firstPublishedAtField as part of the content-type schema json with default value as true anytime a new content type is created
2. Add code to update the schema attribute to include firstPublishedAt field if it is enabled in the schema json (enabled by default)
3. For existing apps, add DB migrations using publishedAt date as the value (similar to draftAndPublish)

For point 1, if possible, can you point me to some code sample where some schema options are being set when a new content type is created?

@alexandrebodin

Got it. Thank you for all the help.

Here is the final action plan:

1. Add code to include firstPublishedAtField as part of the content-type schema json with default value as true anytime a new content type is created
2. Add code to update the schema attribute to include firstPublishedAt field if it is enabled in the schema json (enabled by default)
3. For existing apps, add DB migrations using publishedAt date as the value (similar to draftAndPublish)

For point 1, if possible, can you point me to some code sample where some schema options are being set when a new content type is created?

If you look in packages/core/content-type-builder/server/src/services/schema-builder/content-type-builder.ts you will see where we set the options that will be saved to file :)

@alexandrebodin
Thanks.

Also, just realized one more thing while working on this.

Wouldn't it be better if we create the firstPublishedAt field by default and keep it private/not private since by default we are enabling it anyway.

This way, if someone switches off the feature and later switches it on again, it wouldn't cause problem with the firstPublishedAt value disappearing since while turning it off would not include it in the schema.

What I mean is, if someone disables firstPublishedAt and reruns the app, strapi would remove this from the schema and subsequent enabling would not fetch any value since it was removed when it was disabled earlier.

Wouldn't this cause confusion? Wouldn't it be better we keep it always as part of the schema and show is depending on if it is enabled or not?

I am not sure if I am completely understood this or not. Please let me know if I am missing something here.

@alexandrebodin Thanks.

Also, just realized one more thing while working on this.

Wouldn't it be better if we create the firstPublishedAt field by default and keep it private/not private since by default we are enabling it anyway.

This way, if someone switches off the feature and later switches it on again, it wouldn't cause problem with the firstPublishedAt value disappearing since while turning it off would not include it in the schema.

This would be a breaking change. we need to make sure someone upgrading to the latest Strapi version doesn't get this field added at all. it is not in the reserved names so someone could already have a field call that way

@alexandrebodin Thanks.
Also, just realized one more thing while working on this.
Wouldn't it be better if we create the firstPublishedAt field by default and keep it private/not private since by default we are enabling it anyway.
This way, if someone switches off the feature and later switches it on again, it wouldn't cause problem with the firstPublishedAt value disappearing since while turning it off would not include it in the schema.

This would be a breaking change. we need to make sure someone upgrading to the latest Strapi version doesn't get this field added at all. it is not in the reserved names so someone could already have a field call that way

@alexandrebodin
Makes sense.
So if someone disables it and reenabled it, is it acceptable that the value is gone and it needs to be updated by the user after enabling it again?

Edit:
If someone enables it after disabling it for a while, the migration would pick the value from publishedAt field, which means if someone keeps disabling and enabling it, due to the migrations, the value of firstPublishedAt will keep on updating. Is my understanding correct? Is this the expected behaviour?

Hey@alexandrebodin

Just wanted to check back for the above query in case you missed my previous comment :-)

Here is the scenario that I would need some more information on:

1. User creates a new content type
2. This would create that content type schema with firstPublishedAt as true
3. If firstPublishedAt is true, during strapi start, firstPublishedAt key would be added to the DB schema
4. User fetches the data using the API and gets this firstPublishedAt in the response
5. User makes firstPublishedAt as false in the content type schema and restarts strapi
6. Since, firstPublishedAt is now false, this would remove this from the DB (Correct me if I am wrong here but in my limited knowledge, this is how it should work)
7. User adds some new entries.
8. User enables firstPublishedAt in the content type schema
9. Because it is now enabled and when strapi restarts, the migration would pick it up and populate firstPublishedAt key using publishedAt key (this is required since in step 6, firstPublishedAt was deleted when it was made false in the schema)
10. User sees this new firstPublishedAt value

Is this scenario expected?

Hey@alexandrebodin

Just wanted to check back for the above query in case you missed my previous comment :-)

Here is the scenario that I would need some more information on:

1. User creates a new content type
2. This would create that content type schema with firstPublishedAt as true
3. If firstPublishedAt is true, during strapi start, firstPublishedAt key would be added to the DB schema
4. User fetches the data using the API and gets this firstPublishedAt in the response
5. User makes firstPublishedAt as false in the content type schema and restarts strapi
6. Since, firstPublishedAt is now false, this would remove this from the DB (Correct me if I am wrong here but in my limited knowledge, this is how it should work)
7. User adds some new entries.
8. User enables firstPublishedAt in the content type schema
9. Because it is now enabled and when strapi restarts, the migration would pick it up and populate firstPublishedAt key using publishedAt key (this is required since in step 6, firstPublishedAt was deleted when it was made false in the schema)
10. User sees this new firstPublishedAt value

Is this scenario expected?

Yes this looks good. Objectively someone wouldn't be playing around with the option in production with actual usefull data. once you decide to enable it (for old projects) or disable it for new projects it most likely will stay that way.

One last thing I think you need to check with this though is that the condition to add it should be thatfirstPublishedAt &draftAndPublish are both true. We can't add it for type that disable D&P :)

In terms of the logic to set the value the default() { new Date() } won't be sufficient, it would add the date on the draft data. you will need more logic to be added in the document service.

In terms of the logic to set the value the default() { new Date() } won't be sufficient, it would add the date on the draft data. you will need more logic to be added in the document service.

@alexandrebodin
Got it. Thanks.

I tried playing around with existing data and when firstPublishedAt was present for publishedData and not for draft data, publishing some new content was making firstPublishedAt field null. (Not sure why?)

Adding firstPublishedAt to draft data solved this problem. Will need to check why it was happening. Let me check in the code and try to understand what is going on.

Thanks again.

Hi@alexandrebodin

I have updated the code wherever applicable. I am yet to add the test cases but just wanted to get a quick review to make sure if this is what is expected. 
Would you mind taking a look at the changes? :)

Changes:

• Added firstPublishedAt: true for content-type schema builder
• If firstPublishedAtField is enabled and draftAndPublish are enabled, then added the firstPublishedAt attribute to the schema.
• While publishing new content, added firstPublishedAt but only if it is being added for the first time. (Checked this by checking if there is any old published entry present or not)
• While finally updating firstPublishedAt, also checked if first published at is present as data when request to update this field is coming from the api. This is to make sure that someone can update first published at only when sending as part of the update api otherwise first published at should either be the old one or if not present, it should be same as publishedAt field
• Also, if someone sends a request to update firstPublishedAt, it should not be part of draft but only for the published entry so removed firstPublishedAt when draft is being updated.
• Added migration which checks if the firstPublishedAtField has been enabled, copy the value from publishedAt and copy it to firstPublishedAt

Please note:

1. All existing tests are passing except a few which needs the snapshots to be updated.
2. Will be working on adding the test cases next. Just wanted to make sure if the code changes are in accordance to what is expected :)

Thank you.

Hey@ankit7201 Our team managing this scope will take over the review. Something I have been thinking about that could make more sense is set this as an option in the config folder so it's just a flag to trigger the new fields if D&P is enabled and it will apply everywhere. I'm a bit concerned about have an option per content type :)

The team will be coming back to you soon with some product options :)