Added "Referenced By" and "Depends On" section for each SQL file #106

stephen8chang · 2020-06-25T16:36:20Z

Problem

When one is trying to view SQL files and their immediate relationships to other SQL files in DBT docs, they have to go through a rather annoying process of clicking the lineage graph, and possibly few other clicks just to view the parent or child of the currently selected SQL file.

Solution

To make this process more convenient for the user, this PR adds a "Referenced By" and "Depends On" section in addition to the description of each SQL file, thus allowing the user to view the immediate parent and immediate child files (of any type) of the currently selected SQL file.

To accomplish this, the following changes were made to most files in src/app/docs :

Additional section components were added for "Referenced By" and "Depends On" in the HTML docs files
Added utils.js, which includes the functions to obtain the files that are "Referenced By" or "Depends On" the current SQL file. This is imported into all of the JavaScript files in the docs folder.
In macro.js, unique versions of the above two functions are written, mainly to accommodate for the fact that macro's can only have parents that are other macros (this change does not appear in src/manifest.json)

Other Information

Resolves Add Referenced By and Depends On section to node pages #102
@Matt343 is also a collaborator on this PR

…l docs files

…ed fixed comments and modified overview.html

guy-adams · 2020-06-25T16:42:45Z

FWIW this is a lovely addition - well done!

drewbanin · 2020-06-27T15:25:29Z

hey @stephen8chang - this is AWESOME! I'll try to dig in here today, but just wanted to give you an early heads up that

I really love this and
I'm definitely interested in figuring out how to get this merged

drewbanin · 2020-07-25T16:04:39Z

Thanks for sending through this PR @stephen8chang (and h/t to @Matt343 as well!). I've spent some time clicking around in here today, and I do buy the utility of some new components in the UI for "referenced by" and "depends on" for sure.

I think that at a certain point, we're going to need to rethink the UI/layout of this page. I don't love the fact that our only place for information like this today is inline in the page. Ideally, I'd like to consider some alternative layouts that keep the primary information front-and-center, while still making information like this accessible when it is relevant. @amy-byrum, you think maybe we can use this PR as an opportunity to give some thought to the future of the Docs UI?

All this said, I don't think we need to block merging this PR on a redesign of the Docs site :D. @jtcohen6 adding you for review here. I'll take care of the code review piece, but it would be awesome if you could spend some time clicking around in here and noting your thoughts. I would really like to get this change shipped for 0.18.0 if we can!

Thanks again all - super exciting!

drewbanin

Just dropped a couple of comments in here, happy to discuss!

drewbanin · 2020-07-25T16:08:39Z

src/app/docs/utils.js

@@ -0,0 +1,36 @@
+const _ = require('underscore');
+export function getReferences(project, model) {


We've historically leveraged services for things like this, but I'm increasingly super into the idea of just using a require instead of going through Angular's dependency injection flow.

Can you just rename this from utils.js to something more specific like dag_utils.js instead?

drewbanin · 2020-07-25T16:18:35Z

src/app/docs/macro.js

@@ -15,25 +15,47 @@ angular
    $scope.project = projectService;
    $scope.codeService = codeService;

-    function getReferences(project, macro) {
-        var references = _.filter(project.nodes, function(node) {
+    function getReferences(project, self) {


Do you think there's a way to reconcile this logic with the code in src/app/docs/utils.js? I totally get that the different namespaces for macros and nodes could make this challenging, but it feels funny to have two very similar sets of functions so far apart from each other. Let me know what you think!

The decision to include these two functions in macro.js and separate from utils.js was to accommodate for the differing properties of macros and nodes. Would it make sense to you to have the two functions in macro.js included in utils.js as well?

@drewbanin I moved the functions related to obtaining references and parents for macros from macro.js to dag_utils.js (previously utils.js). Let me know what you think.

drewbanin · 2020-07-25T16:19:48Z

src/app/docs/model.html

@@ -36,6 +36,8 @@ <h1>
                <li ui-sref-active='active'><a ui-sref="dbt.model({'#': 'details'})">Details</a></li>
                <li ui-sref-active='active'><a ui-sref="dbt.model({'#': 'description'})">Description</a></li>
                <li ui-sref-active='active'><a ui-sref="dbt.model({'#': 'columns'})">Columns</a></li>
+                <li ui-sref-active='active'><a ui-sref="dbt.model({'#': 'referenced_by'})">Referenced By</a></li>


We can and should figure out a better way to reduce the copy/paste necessary for these different node types. Not a problem at all for right now though :)

Sounds good! Let me know if you would like me to spend some time on figuring out how to reduce the copy pasting.

jtcohen6 · 2020-08-02T18:50:19Z

This is really cool! I agree with your comments above @drewbanin. Let's think about a better home for this information during a future redesign.

A couple of oddities I noticed:

Models include schema tests in the Referenced By section, as well they should. This is weird only because, while schema tests do populate pages in the docs site, they don't map to anything in the Project, Database, or DAG view. (example). Every other resource maps to one or more of those.
Macros do not appear as each others' dependencies. E.g. dbt_utils.date_spine calls dbt_utils.dateadd, but dbt_utils.dateadd says no resources reference it. That's true in the docs today.
The placeholder text, for both Referenced By and Depends On, appears to be No resources reference this macro, regardless of resource type.

The only change I'd request of the current PR relates to the last point. I think the simplest solution would be to exclude the Referenced By/Depends On section if it's completely empty.

Matt343 · 2020-08-03T21:13:01Z

Macros do not appear as each others' dependencies. E.g. dbt_utils.date_spine calls dbt_utils.dateadd, but dbt_utils.dateadd says no resources reference it. That's true in the docs today.

The reason for this is that the relevant sections in the manifest.json file are always empty for macros. I'm not sure why that is, but depends_on.macros is always empty for a macro node, even if it does contain macro references. To fix this, we'd need to fix that bug in dbt core.

…ro.js -> dag_utils.js

stephen8chang · 2020-08-03T22:09:04Z

@jtcohen6 The References By/Depends On section now appears only if it exists for that specific table.

jtcohen6 · 2020-08-06T17:23:59Z

Re: inter-macro dependencies: it sounds like we're not recording those in the manifest as we should be.

Thanks for making the change around empty Depends On / Referenced By! I definitely like the look better. I think we may now be missing some sections in cases where a model is referenced by or depends on only non-model nodes. In the deploy preview, e.g.

stg_customers should have a "Depends On" for the seed raw_customers
the schema test unique_dim_customers_customer_id depends on dim_customers, but dim_customers does not include a Referenced By section listing that test

…docs into util-changes

stephen8chang · 2020-08-10T16:08:43Z

@jtcohen6 Sorry for the late response! Looks like the issue was with the conditional for the HTML components I had for commit 39345e5. Let me know how it looks on your end.

jtcohen6

Nice! Looks like that resolved it. Thanks so much for the contribution @stephen8chang

Stephen Chang added 6 commits June 22, 2020 16:00

Initial changes - added child and parent files for each sql file

6873bdb

Overview header fix and removal of getParents and getReferences in al…

a413f1a

…l docs files

Deleted data/manifest.json

1a282b9

Adjusted scope of certain variables in docs

4698f1a

Changed scope of variables in utils.js

c1e6aff

Added back data/manifest.json, changed "require" for some docs, delet…

76dc447

…ed fixed comments and modified overview.html

Stephen Chang added 2 commits June 26, 2020 13:18

Added "Referenced By" and "Depends On" to files of type source

bf574c8

Added hyperlink tabs ("Referenced By" and "Depends On") for source files

b1c53b0

drewbanin requested review from drewbanin and jtcohen6 July 25, 2020 16:04

drewbanin reviewed Jul 25, 2020

View reviewed changes

Excludes RB and/or DO section if it does not exist

39345e5

Stephen Chang added 2 commits August 3, 2020 16:29

Changed name of utils.js -> dag_utils.js and moved functions from mac…

4f86531

…ro.js -> dag_utils.js

Changes in variable naming convention for dag_utils import

e7c984b

Merge branch 'master' into util-changes

e1662ca

Stephen Chang added 3 commits August 6, 2020 16:55

Fixed conditional for excluding RB/DO sections if they are empty

ca1b145

Additional modifications after recent git pull

d023bb5

Merge branch 'util-changes' of https://github.com/insurancezebra/dbt-…

522207d

…docs into util-changes

jtcohen6 approved these changes Aug 11, 2020

View reviewed changes

jtcohen6 merged commit 13feab1 into dbt-labs:master Aug 11, 2020

This was referenced Aug 14, 2020

Docs changes for 0.18.0 dbt-labs/dbt-core#2705

Closed

Docs site updates for 0.18.0 dbt-labs/dbt-core#2715

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added "Referenced By" and "Depends On" section for each SQL file #106

Added "Referenced By" and "Depends On" section for each SQL file #106

stephen8chang commented Jun 25, 2020 •

edited

Loading

guy-adams commented Jun 25, 2020

drewbanin commented Jun 27, 2020

drewbanin commented Jul 25, 2020

drewbanin left a comment

drewbanin Jul 25, 2020

stephen8chang Jul 30, 2020

drewbanin Jul 25, 2020

stephen8chang Jul 30, 2020

stephen8chang Aug 3, 2020

drewbanin Jul 25, 2020

stephen8chang Jul 30, 2020

jtcohen6 commented Aug 2, 2020 •

edited

Loading

Matt343 commented Aug 3, 2020

stephen8chang commented Aug 3, 2020

jtcohen6 commented Aug 6, 2020 •

edited

Loading

stephen8chang commented Aug 10, 2020

jtcohen6 left a comment

		@@ -0,0 +1,36 @@
		const _ = require('underscore');
		export function getReferences(project, model) {

Added "Referenced By" and "Depends On" section for each SQL file #106

Added "Referenced By" and "Depends On" section for each SQL file #106

Conversation

stephen8chang commented Jun 25, 2020 • edited Loading

Problem

Solution

Other Information

guy-adams commented Jun 25, 2020

drewbanin commented Jun 27, 2020

drewbanin commented Jul 25, 2020

drewbanin left a comment

Choose a reason for hiding this comment

drewbanin Jul 25, 2020

Choose a reason for hiding this comment

stephen8chang Jul 30, 2020

Choose a reason for hiding this comment

drewbanin Jul 25, 2020

Choose a reason for hiding this comment

stephen8chang Jul 30, 2020

Choose a reason for hiding this comment

stephen8chang Aug 3, 2020

Choose a reason for hiding this comment

drewbanin Jul 25, 2020

Choose a reason for hiding this comment

stephen8chang Jul 30, 2020

Choose a reason for hiding this comment

jtcohen6 commented Aug 2, 2020 • edited Loading

Matt343 commented Aug 3, 2020

stephen8chang commented Aug 3, 2020

jtcohen6 commented Aug 6, 2020 • edited Loading

stephen8chang commented Aug 10, 2020

jtcohen6 left a comment

Choose a reason for hiding this comment

stephen8chang commented Jun 25, 2020 •

edited

Loading

jtcohen6 commented Aug 2, 2020 •

edited

Loading

jtcohen6 commented Aug 6, 2020 •

edited

Loading