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

Incorrect FIRST_CLOSE_AT on intercom__conversation_metrics #40

Closed
2 of 4 tasks
alvarosiman opened this issue Apr 12, 2023 · 15 comments
Closed
2 of 4 tasks

Incorrect FIRST_CLOSE_AT on intercom__conversation_metrics #40

alvarosiman opened this issue Apr 12, 2023 · 15 comments
Assignees
Labels
status:in_review Currently in review type:enhancement New functionality or enhancement update_type:models Primary focus requires model updates

Comments

@alvarosiman
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Describe the issue

Please refer to ticket #125497 as it was the first reported issue we found with Fivetran's Intercom packages. After months of troubleshooting we finally found the issue with the package and Fivetran fixed.

From Intercom data we can clearly see that time of first close for the conversation with ID 38756 was 19:23 UTC but Fivetran's package is showing the next day at 22:08 UTC (the last close instead of the first). Could we please fix.

image

Relevant error log or model output

No response

Expected behavior

The expected behavior is to grab the first timestamp the conversations was closed

image

dbt Project configurations

Intercom

intercom_database: fivetran
intercom_schema: intercom
intercom__using_team: False
intercom__using_company_tags: False

Package versions

packages:
# dbt_date, dbt_expectations, codegen can all be upgraded
# but are blocked by github which has a dependency on
# dbt_utils [">=0.8.0", "<0.9.0"].

  • package: fivetran/github
    version: [">=0.7.0", "<0.8.0"]
  • package: dbt-labs/metrics
    version: [">=1.3.0", "<1.4.0"]
  • package: fivetran/app_reporting
    version: [">=0.2.0", "<0.3.0"]
  • package: fivetran/intercom
    version: [">=0.6.0", "<0.7.0"]
  • package: fivetran/iterable
    version: [">=0.6.0", "<0.7.0"]

What database are you using dbt with?

snowflake

dbt Version

image

Additional Context

No response

Are you willing to open a PR to help address this issue?

  • Yes.
  • Yes, but I will need assistance and will schedule time during our office hours for guidance
  • No.
@fivetran-joemarkiewicz fivetran-joemarkiewicz added the status:scoping Currently being scoped label Apr 13, 2023
@fivetran-joemarkiewicz
Copy link
Contributor

Hi @alvarosiman thanks so much for opening this issue and raising the incorrect first close to our team.

When looking at our code to calculate first_close_at I see that we are grabbing the first created_at field for the filtered results in the cte where part_type = 'close' and author_type = 'admin'. Are you able to confirm if the close entry you are highlighting was done by an admin?

first_value(created_at) over (partition by conversation_id order by created_at asc, conversation_id rows unbounded preceding) as first_close_at,
first_value(created_at) over (partition by conversation_id order by created_at desc, conversation_id rows unbounded preceding) as last_close_at
from conversation_part_history
where part_type = 'close' and author_type = 'admin'

If it was not closed by an admin I would be curious to get your thoughts if that should still be counted as a first close time?

@alvarosiman
Copy link
Author

alvarosiman commented Apr 13, 2023

Hi Joe, thanks for your reply!

I see, that's what I thought was happening. Based on how Fivetran packages treat Intercom data it would make sense to have a first_admin_close_at similar to the first_admin_response_at and last_admin_response_at

Intercom has many new functionalities, including very useful bots that can close conversations if an admin has responded and the end customer has been inactive after x amount of minutes.
This is actually the case for most of our conversations. The user starts it, the admin responds and provides a solution or asks them a follow up question, but the end user doesn't respond again. Since having the admin close after responding would not make sense if the conversation isn't clearly finished, Intercom's bot asks the user if they are still there and closes the conversation telling them they can re-open it if they want to continue.

So in general it would be great to have a first_admin_close_at and a first_close_at regardless of if it was an admin

@fivetran-joemarkiewicz
Copy link
Contributor

Thanks for the quick reply!

I was not aware of the bots Intercom may deploy to help automate these types of conversation parts (makes sense to me!). I would also agree with you that expanding the model to have two different versions of close_at can help users who leverage bots in their workflows. Would it be difficult on your end to manage these two fields when analyzing the data?

If not, then I feel this is an appropriate update to the package and can fold this into our upcoming sprint.

@alvarosiman
Copy link
Author

I think there would be no issue on our side managing the two separate fields, it would make our analytics more accurate and give us more flexibility.

That would be great! When would the next sprint finish? And would we just have to update the package version to get access to this? Thanks!

@fivetran-joemarkiewicz
Copy link
Contributor

Our next sprint begins Wednesday of next week. As this is likely a relatively small update we should be able to turn this around in the first half of the sprint.

Yup! Once the update is applied you will be able to install the newest version of the package to capture the changes. As we will likely be adding a new field and changing the definition of the first_close_at field, I would be most comfortable with this being a breaking change. So the next version will be v0.7.0.

Additionally, we will share a WIP branch here before releasing in case you wanted to validate on your end that changes look good before making the official release.

@fivetran-joemarkiewicz fivetran-joemarkiewicz added status:accepted Scoped and accepted into queue and removed status:scoping Currently being scoped labels Apr 13, 2023
@alvarosiman
Copy link
Author

Thanks Joe! When do you think we will we be able to validate on our side? and if it's taken on the first half of the sprint would it be released by the end of this week or the next?

@fivetran-joemarkiewicz
Copy link
Contributor

Hi @alvarosiman I think conservatively we should have a working branch over to you to test by the end of this week. Realistically, this will be released early next week if all goes well during validation.

@alvarosiman
Copy link
Author

Great, thanks for the update!

@fivetran-joemarkiewicz fivetran-joemarkiewicz added type:enhancement New functionality or enhancement status:in_progress Currently being worked on update_type:models Primary focus requires model updates and removed status:accepted Scoped and accepted into queue labels Apr 20, 2023
@fivetran-joemarkiewicz
Copy link
Contributor

Hi @alvarosiman I wanted to post back that we currently have a working version of the package with the changes discussed earlier in this thread. If you have a moment, it would be great if you would be able to validate the changes on your end and confirm they make sense and are accurate.

You can test the changes by replacing your existing package dependency for intercom within your packages.yml with the following dependency:

packages:
  - git: https://github.com/fivetran/dbt_intercom.git
    revision: feature/close-at-updates
    warn-unpinned: false 

You can see a full view of the updates applied in this branch by inspecting the notes and files changed within PR #41

Let me know if you have any questions. Thanks!

@alvarosiman
Copy link
Author

Thanks @fivetran-joemarkiewicz

Will take a look and get back to you.

@alvarosiman
Copy link
Author

Hi @fivetran-joemarkiewicz! Hope you are doing well. I checked a couple of runs and the information seems to be accurate. Thanks!

@fivetran-joemarkiewicz
Copy link
Contributor

Brilliant! Thanks for checking. I will kick off the review process internally and will share once this is live.

@fivetran-joemarkiewicz fivetran-joemarkiewicz added status:in_review Currently in review and removed status:in_progress Currently being worked on labels Apr 24, 2023
@alvarosiman
Copy link
Author

Hi @fivetran-joemarkiewicz hope everything is well! Is there an update on this PR? Do you think it will get released soon?

@fivetran-joemarkiewicz
Copy link
Contributor

Hi @alvarosiman this is currently in the internal release process. I imagine you can expect this to be released today!

@fivetran-joemarkiewicz
Copy link
Contributor

Hi @alvarosiman the changes from the branch I linked above are now live on the dbt hub! You can upgrade your package to the latest version (v0.7.0) in order to capture these changes.

As such, I will close this issue. Please feel free to reopen if the issue continues to persist on your end. Thanks again for helping work through this update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:in_review Currently in review type:enhancement New functionality or enhancement update_type:models Primary focus requires model updates
Projects
None yet
Development

No branches or pull requests

2 participants