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

[WIP] Allow user to add a name to a deployment so that in history it can be… #359

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

joiecosby
Copy link
Collaborator

… more easily identified 

Adding new functionality that will allow users to save deployments. This should be accessible from all deployment options

resolves #331

… more easily identified 

Adding new functionality that will allow users to save deployments. This should be accessible from all deployment options

resolves #331
@nx-cloud
Copy link

nx-cloud bot commented May 14, 2023

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 88049fe. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@joiecosby
Copy link
Collaborator Author

@paustint I am curious what opinions you have on the current build so far. At the moment, I have it working but it feels disjointed. I have worked it out for one scenario, the Deploy to Org button.

This is how the logic gets called for displaying the deployment options:
DeploymentMetadataToOrg --> DeploymentMetadataPackageConfigModal --> DeploymentOptions

The logic gets called for displaying the deployment status:
DeploymentMetadataToOrg --> DeployMetadataPackageStatusModal --> DeployMetadataStatusModal

The logic gets passed to and from DeploymentMetadataToOrg but I can't tell if there is a better way.

I am also running into the following error for DeployMetadataToOrgConfigModal:

Screen Shot 2023-05-15 at 19 13 32

And into this error in DeployOptions:
Screen Shot 2023-05-15 at 19 46 06

Thank you in advance!

@paustint
Copy link
Contributor

@joiecosby - I didn't review everything in full detail, but here are some thoughts:
It really does not feel right to add deploymentHistoryName to export interface DeployResult { since that should truly represent the data that Salesforce returns - I think we can remove this.

I would probably do something like this (I did not do a full analysis, just a cursory glance)

Basically just store the name locally wherever it is needed, and then pass it along with the function call to initiate the deployment (basically everywhere saveHistory({ is called)

// deploy-metadata.utils.tsx
export async function saveHistory({
...
deployOptions: DeployOptions;
+/** Optional name to use for history */
+name?: string;
...
}) {

// there are a few places very similar to this
// useDeployMetadata....tsx
-const deployMetadata = useCallback(async () => {
+const deployMetadata = useCallback(async (deploymentHistoryName?: string) => {
saveHistory({
...
+name: deploymentHistoryName,
...
});

@joiecosby
Copy link
Collaborator Author

Notes from pair programming:

  • Change defining deploymentHistoryName as string | null and instead define as Maybe
  • Remove logic adding deploymentHistoryName to DeployResults instead wherever saveHistory() is being referenced, add the logic inside there
  • Make sure that we are including the logic for change set scenario and delete metadata scenario.

<Input label="Deployment History Name" className="slds-grow">
<input
className="slds-input"
value={undefined}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@paustint do you have a suggestion on how to not automatically pass this as undefined?

When I navigate away from the modal and back, I ideally want to have the name the user inputted.

Copy link
Contributor

Choose a reason for hiding this comment

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

(unrelated) for an input, you never want the value to start as undefined otherwise you will get an error saying "input is changing from uncontrolled to controlled"

Just like you are passing in setDeploymentHistoryName, you also need to pass in the value you want to use as the initial state (and this should start as empty string, not undefined)

Copy link

nx-cloud bot commented Mar 21, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit cd77497. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


🟥 Failed Commands
nx run-many --target=build --parallel=3 --projects=jetstream,api,download-zip-sw,landing --configuration=production

Sent with 💌 from NxCloud.

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.

Allow user to add a name to a deployment so that in history it can be more easily identified
2 participants