Closed fivetran-catfritz closed 10 months ago
@fivetran-joemarkiewicz Thanks for reviewing! I have pushed the updates to remove the else
s. As for other custom fields, the customer did run with a ton of the columns listed in the issue (see the dbt project configs), and said there weren't any other affected fields.
However now that I think of this more, I'm still a little uncertain about what they are supposed to display. I tested out some additional fields like resolution
, priority
and project
, and they display their id rather than a name. However, I updated the issue_type to show the name rather than id. Is this actually what it's supposed to be?
However now that I think of this more, I'm still a little uncertain about what they are supposed to display. I tested out some additional fields like
resolution
,priority
andproject
, and they display their id rather than a name. However, I updated the issue_type to show the name rather than id. Is this actually what it's supposed to be?
@fivetran-catfritz yup this is what I was leaning towards with my probing question around if other custom fields need a similar update. The ultimate goal of this jira__daily_issue_field_history
model is to display the latest field history (of those selected) value for an issue on a given day. This model is primarily generated by the stg_jira__issue_field_history
model as well as a few others. The end model consolidates, pivots, and creates a slowly changing dimension from that staging model. In that staging model the values for some of the fields are foreign keys to tables which the connector syncs. This is what we saw originally with components
and then status
, and now what you are seeing with issue_type. I was curious if this is also the case for other field types that have their own tables (ie. priority
, project
, resolution
) and that does seem to be the case. With this, I would recommend opening a FR on this repo to expand the logic we have for components, status, and issue type to include the other fields with a similar FK relationship.
You can see that this is not the case for fields that do not have their own tables synced via the connector. Here is an example of my including a field that has a FK (shows as and ID) and one that does not. Notice how summary
is not an ID but the actual value since there is no raw summary table. Ideally, we could create an end model that includes the value as opposed to the ID for the fields with their own tables. However, that is out of scope for this ticket and something we should handle in a future update.
Thanks @fivetran-joemarkiewicz that makes sense now. I found this issue already exists, so I'll add comments with more detail of what we found this round.
I would say this is ready for re-review now too!
PR Overview
This PR will address the following Issue/Feature: #107
This PR will result in the following new package version:
Please detail what change(s) this PR introduces and any additional information that should be known during the review of this PR:
Updated the
jira__daily_issue_field_history
model to make sureissue_type
values are correctly joined into the downstream issue models. This applied only ifissue type
is leveraged within theissue_field_history_columns
variable.Reorganized some of the logic in
jira__daily_issue_field_history
so that the for loops run less often and used nested conditionals.PR Checklist
Basic Validation
Please acknowledge that you have successfully performed the following commands locally:
Before marking this PR as "ready for review" the following have been applied:
Detailed Validation
Please acknowledge that the following validation checks have been performed prior to marking this PR as "ready for review":
Standard Updates
Please acknowledge that your PR contains the following standard updates:
dbt Docs
Please acknowledge that after the above were all completed the below were applied to your branch:
If you had to summarize this PR in an emoji, which would it be?
:dancer: