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

Update putDataSource to handle removing description. #1765

Merged
merged 2 commits into from Oct 25, 2023

Conversation

mknowlton89
Copy link
Collaborator

@mknowlton89 mknowlton89 commented Oct 18, 2023

Features and Changes

This PR updates the putDataSource method. Initially, this update was just to resolve a bug where the description of a data source object wasn't able to be removed. With the way the code was written, we were only updating the datasource description if a description was passed in - and when removing the description, an empty string is passed in, so the if (datasource) check was returning false, and thus skipping the code that actually updates the description.

In looking at the endpoint further, we're actually always passing in a full datasource object (save the dateCreated/dateUpdated fields), so all of the checks in the method that were checking for various properties were unnecessary.

This PR removes the unneeded checks and simplifies the method.

Testing

  • Confirm that if you remove a description from a data source via the DataSourceForm, the description is actually removed.
  • Confirm no new regressions are introduced with this refactor
  • Confirm the ability to create a data source still works via this form
  • Confirm the ability to edit the name of a data source still works via this form
  • Confirm the ability to edit multiple fields on this data source object still works via this form

@@ -428,11 +428,11 @@ export async function putDataSource(
req: AuthRequest<
{
name: string;
description?: string;
description: string;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is only one place in the app where we're hitting this endpoint, and we're passing in the full "Datasource" object, not just the updates, so description will always be provided, it may just be an empty string. Same with projects

Copy link
Member

Choose a reason for hiding this comment

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

Is this true? It looks like it's also called in [did].tsx where only settings and nothing else is passed in.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know how I missed that. I've updated this PR in light of this. I mostly reset everything, and when it comes to the description, I'm checking to see if the description property exists in the req.body, and if so, then setting using the value to update the description.

@github-actions
Copy link

Your preview environment pr-1765-bttf has been deployed.

Preview environment endpoints are available at:

@mknowlton89 mknowlton89 marked this pull request as ready for review October 19, 2023 10:55
…object not being passed in on the [did].tsx component.
@mknowlton89 mknowlton89 self-assigned this Oct 24, 2023
@mknowlton89 mknowlton89 merged commit 6585387 into main Oct 25, 2023
3 checks passed
@mknowlton89 mknowlton89 deleted the mk/edit-description-bug branch October 25, 2023 12:55
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.

[Bug] Unable to remove data source description
2 participants