fix(model): fixes DashboardContainsDashboard relationship in DashboardInfo aspect

[sgomezvillamor]

This fixes:

• what is most likely a copy-paste issue in the DashboardContainsDashboard relationship model for DashboardInfo aspect model
• a wrong validation in the DashboardPatchBuilder#add_chart_edge
• adds the missing DashboardPatchBuilder#add_dashboard

Checklist

• The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
• Links to related issues (if applicable)
• Tests for the changes have been added/updated (if applicable)
• Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
• For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

[codecov]

Codecov Report

Attention: Patch coverage is 57.14286% with 21 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...spect/patch/builder/DashboardInfoPatchBuilder.java	0.00%	10 Missing ⚠️
...tadata-ingestion/src/datahub/specific/dashboard.py	61.53%	5 Missing ⚠️
...oreindices/dashboardinfo/ReindexDashboardInfo.java	66.66%	2 Missing and 1 partial ⚠️
...ndices/dashboardinfo/ReindexDashboardInfoStep.java	50.00%	3 Missing ⚠️

Files with missing lines	Coverage Δ
...fig/restoreindices/ReindexDashboardInfoConfig.java	100.00% <100.00%> (ø)
...atch/template/dashboard/DashboardInfoTemplate.java	95.45% <100.00%> (+0.71%)	⬆️
...atahub/ingestion/api/incremental_lineage_helper.py	86.95% <100.00%> (+2.68%)	⬆️
...oreindices/dashboardinfo/ReindexDashboardInfo.java	66.66% <66.66%> (ø)
...ndices/dashboardinfo/ReindexDashboardInfoStep.java	50.00% <50.00%> (ø)
...tadata-ingestion/src/datahub/specific/dashboard.py	56.86% <61.53%> (+6.86%)	⬆️
...spect/patch/builder/DashboardInfoPatchBuilder.java	0.00% <0.00%> (ø)

... and 21 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 47134c2...08bb142. Read the comment docs.

[mayurinehate]

Looks reasonable fix.
Hoping that re-ingestion of this aspect will automatically update graph index. Wondering if you've tested this.

[sgomezvillamor]

Hoping that re-ingestion of this aspect will automatically update graph index. Wondering if you've tested this.

Not tested and not sure how to test

[hsheth2]

Can you add a note to the updating-datahub.md doc to call out that users of Sigma and PowerBI Apps ingestion may need to run a targeted reindex if they need updated metadata on these edges

I doubt there's many cases of that, but would be good to mention anyways

[hsheth2]

LGTM

Might be worth getting a quick look from @RyanHolstien

[RyanHolstien]

I think typically we add an upgrade job that runs once to reindex these relationships when we update them, would also like to see some Java side unit tests

[RyanHolstien]

Let's add tests for these

[sgomezvillamor]

I found these tests for DashboardInfoPatchBuilder

datahub/metadata-integration/java/datahub-client/src/test/java/datahub/client/patch/PatchTest.java

Line 619 in 79aa40f

new DashboardInfoPatchBuilder()

I will add it there, even if they are all @Ignore.

[sgomezvillamor]

addressed in commit 49e0dc2

[RyanHolstien]

If there isn't already a test for this let's create one, or if there is add in mapping this field

[sgomezvillamor]

I had already added a test for DashboardInfoTemplate in https://github.com/datahub-project/datahub/pull/12433/files#diff-b6808c495535fbf32144cbc4e6f16eb762b55c377c63ef929758d21760c28437

The test operates on applyPatch method, which goes through both transformFields and rebaseFields methods, as can be seen in

datahub/entity-registry/src/main/java/com/linkedin/metadata/aspect/patch/template/Template.java

Lines 51 to 86 in 47134c2

	default T applyPatch(RecordTemplate recordTemplate, JsonPatch jsonPatch)
	throws JsonProcessingException {
	TemplateUtil.validatePatch(jsonPatch);
	JsonNode transformed = populateTopLevelKeys(preprocessTemplate(recordTemplate), jsonPatch);
	try {
	// Hack in a more efficient patcher. Even with the serialization overhead 140% faster
	JsonObject patched =
	jsonPatch.apply(
	Json.createReader(new StringReader(OBJECT_MAPPER.writeValueAsString(transformed)))
	.readObject());
	JsonNode postProcessed = rebaseFields(OBJECT_MAPPER.readTree(patched.toString()));
	return RecordUtils.toRecordTemplate(getTemplateType(), postProcessed.toString());
	} catch (JsonProcessingException e) {
	throw new RuntimeException(
	String.format(
	"Error performing JSON PATCH on aspect %s. Patch: %s Target: %s",
	recordTemplate.schema().getName(), jsonPatch, transformed.toString()),
	e);
	}
	}
	/**
	* Returns a json representation of the template, modified for template based operations to be
	* compatible with patch semantics.
	*
	* @param recordTemplate template to be transformed into json
	* @return a {@link JsonNode} representation of the template
	* @throws JsonProcessingException if there is an issue converting the input to JSON
	*/
	default JsonNode preprocessTemplate(RecordTemplate recordTemplate)
	throws JsonProcessingException {
	T subtype = getSubtype(recordTemplate);
	JsonNode baseNode = OBJECT_MAPPER.readTree(RecordUtils.toJsonString(subtype));
	return transformFields(baseNode);
	}

[sgomezvillamor]

I think typically we add an upgrade job that runs once to reindex these relationships when we update them, would also like to see some Java side unit tests

@RyanHolstien I have added the requested reindex job for the upgrade in commit b582b47

Considering we are fixing the DashboardContainsDashboard relationship in DashboardInfo aspect https://github.com/datahub-project/datahub/pull/12433/files#diff-6ae54ee7e3babcd8ac0b99538e8de6725f9fd82c0a119e517c20b2a23575e303 , can you confirm that DashboardInfo aspect reindexation will fix the relationship? or, how could I test and check this?

[david-leifker]

lgtm!