Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add ability to update the target guid for a deployment at any time (from the ... menu) #2576
base: main
Are you sure you want to change the base?
Add ability to update the target guid for a deployment at any time (from the ... menu) #2576
Changes from 2 commits
9ec3b19
c9b2057
a6250d9
90671c6
309ed31
44c4534
fe2e0ee
96043f5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 shows the context menu option while a pre-deployment shows the association link.
We could try to hide it using message passing or we could generalize the label to remove the "with a Different Deployment". 🤔
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.
I'm not following you. Can you provide a screenshot? I'm looking at

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.
Yeah that screenshot shows what I'm talking about. We have both "update that previous deployment" available as a link, and the "Associate with a Different Deployment on Connect". The wording is just strange since it isn't associated with any deployment yet. That may be too in the weeds with how this is working though.
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.
I've added another context state within the extension, which indicates if the actively selected content record is a PreContentRecord or not. This allows the menu item to now be shown only when it has been deployed, which should address the confusion you identified here.
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.
Thanks for doing that, it is a small thing, but it should make it easier to understand for users.
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 is always sending back
true
. Looks like it might be a missing.value
onserverCredential
.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.
I don't understand the purpose of the message sending however. This is checking if the current deployment has a credential, but if we are associating with a different piece of content we could be pointing to an entirely different server. What is this supposed to be catching?
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.
The purpose is to enable or disable the ability to associate, within the ... menu. I've seen it work, so I'm surprised there is an error there, but I'll check it out. What it implements is that when we do not have a credential for the server, so we couldn't contact the server to patch the deployment file, so it would fail always. So the option isn't offered.
I'll check the logic, like I mentioned, I'm not sure with this how I was seeing the vscode context set correctly per the state, as shown in the screen shots.
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.
Updated to examine the reference's value rather than the reference itself. Thanks for catching this. The message stayed in place, as my explanation of its purpose above still stands.