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

Fixes and changes for the data dashboard backend #2159

Merged
merged 7 commits into from
Dec 17, 2024
Merged

Conversation

caiosba
Copy link
Contributor

@caiosba caiosba commented Dec 17, 2024

Description

Some fixes and changes for the data dashboard backend:

  • CV2-5812: Expand the "matched media" data point to include all tipline request types.
  • CV2-5813: Use the default language when an explainer is created without language (includes migration rake task).
  • CV2-5842: Use updated_at instead of created_at for article tags analytics.
  • CV2-5835: For messages analytics, include only the states sent and received.
  • CV2-5836: Use the top_media_tags method from CheckDataPoints lib in the TeamStatistics lib.

How has this been tested?

Existing tests were updated to cover the changes.

Checklist

  • I have performed a self-review of my own code
  • I have added unit and feature tests, if the PR implements a new feature or otherwise would benefit from additional testing
  • I have added regression tests, if the PR fixes a bug
  • I have added logging, exception reporting, and custom tracing with any additional information required for debugging
  • I considered secure coding practices when writing this code. Any security concerns are noted above.
  • I have commented my code in hard-to-understand areas, if any
  • I have made needed changes to the README
  • My changes generate no new warnings
  • If I added a third party module, I included a rationale for doing so and followed our current guidelines

@caiosba caiosba removed the request for review from DGaffney December 17, 2024 02:17
Copy link
Contributor

@melsawy melsawy left a comment

Choose a reason for hiding this comment

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

A minor adjustment regarding the rake task to improve execution time

Copy link
Contributor

Choose a reason for hiding this comment

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

I recommend implementing a loop based on teams, where each team will have a single query to update the Explainer with a nil value and retrieve the language only once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@melsawy , I left it that way because on live there are just around 200 cases... do you think the change is still needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I thought we had many explainers on live

@caiosba caiosba requested a review from melsawy December 17, 2024 13:12
@caiosba caiosba merged commit 621e947 into develop Dec 17, 2024
11 checks passed
@caiosba caiosba deleted the fixes/data-dashboard branch December 17, 2024 16:02
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.

3 participants