posit-dev / publisher

MIT License
3 stars 0 forks source link

Preflight type changes #1755

Closed mmarchetti closed 1 month ago

mmarchetti commented 1 month ago

Intent

This PR adds a preflight check for changes in configuration type. This is an error because Connect can't change the type of a content item once it has been deployed.

Fixes #1666 Fixes #1741

Type of Change

Automated Tests

There isn't a test case that attempts this yet.

Directions for Reviewers

Deploy a content item. Change the type in the configuration file and try to deploy again.

Also try deploying without changing the type.

sagerb commented 1 month ago

This worked great for me with the additional changes. Re-deploying didn't change the record so I wasn't able to change the type with multiple re-deployment attempts. The error was a lot more clear as well.

I do see an updated deployment file after redeplloyment... So this works great.

sagerb commented 1 month ago

I'll add the new field into the client API's within this dependent issue: https://github.com/posit-dev/publisher/issues/1658

dotNomad commented 1 month ago

Should the deployment file be updated if the content type isn't recorded, so we can always expect it?

If the content-type isn't recorded it makes sense to record it. Was there a case you were seeing it not being updated for this PR?

mmarchetti commented 1 month ago

Is this ready to go, or are there still changes needed?

dotNomad commented 1 month ago

Is this ready to go, or are there still changes needed?

It worked for me after additional commits preceding Bill's review. @sagerb did you have any concerns with this merging?

sagerb commented 1 month ago

No concerns, my PR is based off of it. I'm merging.