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

Add Creator Sentiment Email task #38787

Merged
merged 6 commits into from Feb 20, 2024
Merged

Add Creator Sentiment Email task #38787

merged 6 commits into from Feb 20, 2024

Conversation

qwef
Copy link
Contributor

@qwef qwef commented Feb 14, 2024

Closes #38739

Demo

image

@qwef qwef requested a review from camsaul as a code owner February 14, 2024 19:32
@qwef qwef changed the title Add Creator Sentiment Email tasks Add Creator Sentiment Email task Feb 14, 2024
@metabase-bot metabase-bot bot added the .Team/AdminWebapp Admin and Webapp team label Feb 14, 2024
Copy link

replay-io bot commented Feb 14, 2024

Status Complete ↗︎
Commit 80e8ccc
Results
⚠️ 4 Flaky
2313 Passed

Copy link
Contributor

@brunobergher brunobergher left a comment

Choose a reason for hiding this comment

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

Small things to tweak and a reuse suggestion,

src/metabase/email/creator_sentiment_email.mustache Outdated Show resolved Hide resolved
src/metabase/task/creator_sentiment_emails.clj Outdated Show resolved Hide resolved
@qwef qwef added the no-backport Do not backport this PR to any branch label Feb 15, 2024
@qwef qwef requested a review from a team February 15, 2024 23:55
Copy link
Member

@noahmoss noahmoss left a comment

Choose a reason for hiding this comment

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

looks good, just nits & small comments

src/metabase/analytics/stats.clj Outdated Show resolved Hide resolved
src/metabase/email/creator_sentiment_email.mustache Outdated Show resolved Hide resolved
src/metabase/email/messages.clj Outdated Show resolved Hide resolved
src/metabase/email/messages.clj Outdated Show resolved Hide resolved

(defn- fetch-num-dashboards []
(->
(t2/query-one {:select [[:%count.* :num_dashboards]]
Copy link
Member

Choose a reason for hiding this comment

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

can you use t2/count for these simpler count queries?

src/metabase/task/creator_sentiment_emails.clj Outdated Show resolved Hide resolved
qwef and others added 2 commits February 19, 2024 20:35
Co-authored-by: Noah Moss <32746338+noahmoss@users.noreply.github.com>
@qwef qwef merged commit a634189 into master Feb 20, 2024
105 checks passed
@qwef qwef deleted the jh-creator-sentiment-email branch February 20, 2024 18:56
Copy link

@qwef Did you forget to add a milestone to the issue for this PR? When and where should I add a milestone?

@qwef qwef added this to the 0.50 milestone Feb 20, 2024
@dpsutton dpsutton added backport Automatically create PR on current release branch on merge and removed no-backport Do not backport this PR to any branch labels Mar 4, 2024
@dpsutton dpsutton modified the milestones: 0.50, 0.49 Mar 4, 2024
github-actions bot pushed a commit that referenced this pull request Mar 4, 2024
* Add new email function and send email to creator

* remove debug stuff

* add tests, address comments

* actually add tests

* Update src/metabase/email/messages.clj

Co-authored-by: Noah Moss <32746338+noahmoss@users.noreply.github.com>

* address comments

---------

Co-authored-by: Noah Moss <32746338+noahmoss@users.noreply.github.com>
metabase-bot bot added a commit that referenced this pull request Mar 4, 2024
* Add new email function and send email to creator

* remove debug stuff

* add tests, address comments

* actually add tests

* Update src/metabase/email/messages.clj



* address comments

---------

Co-authored-by: Jerry Huang <34140255+qwef@users.noreply.github.com>
Co-authored-by: Noah Moss <32746338+noahmoss@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Automatically create PR on current release branch on merge .Team/AdminWebapp Admin and Webapp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

measuring creator sentiment (BE)
4 participants