Better error message for new new data sources #35679

mkangia · 2025-01-24T19:11:01Z

Product Description

Show user a better message to the user when previewing a data source if its missing or yet to be created.
Additionally, disable the preview button itself

Before:

After:

On Creating a new data source:

Tooltip revealed on clicking the preview button (till replaced with until):

Technical Summary

https://dimagi.atlassian.net/browse/SC-4091

We do have a catch all for programming errors, from where we get the current message we show to the user.
Instead of modifying that to tell a user to wait for new data source to be created, chose to add a message specifically if the table is missing for a better user experience.
Since preview is a rare operation, checking for the table to be present should not add considerable load to the environment.
We can only consider creating the table if its missing when a data source is created. Currently the responsibility of creating new data sources is with pillows when they bootstrap every 4 hours.

Feature Flag

USER_CONFIGURABLE_REPORTS

Safety Assurance

Safety story

Tested locally that message shows well on the preview view which is the only one that is impacted.
For cases where table exists, things work well as before.

Automated test coverage

QA Plan

Not needed.

Rollback instructions

This PR can be reverted after deploy with no further considerations

Labels & Review

Risk label is set correctly
The set of people pinged as reviewers is appropriate for the level of risk of the change

Charl1996 · 2025-01-27T08:01:49Z

corehq/apps/userreports/views.py

-            'data': [list(row) for row in q[:20]],
-            'total_rows': q.count(),
+            'data': data,
+            'total_rows': q.count() if data else 0,


Will q.count not show 0 if the table does not exist, or does it fail?

It fails if table is missing hence the catch to return 0 instead.

ajeety4 · 2025-01-27T08:53:58Z

corehq/apps/userreports/views.py

+            messages.error(
+                self.request,
+                _('Data source table not found!'
+                  '<br/>If you have recently added the data source, please wait for it to be created.'


I think the error also occurs when a user updates the data source like adds a new column.
maybe update the message to reflect that as well ? Something like below

If you have recently added or modified the data source, please wait for operation to be completed.

Hey @ajeety4

Let me try that.
My understanding is that if the table is modified, it will only be deleted later by pillows, and so will be present during preview.
In case there is an error caused due to the change causing an error in the sql query, then it will be caught by the catch all error I mentioned in the description.
I will try and confirm this.

Let me know your follow up thoughts.

I don't have good understanding of role of pillows in datasource creation or modification especially in what situation is triggered.

One of the situation that preview shows the above error is when user modifies the data source, rebuilds the data source and previews it before completion.

Hey @ajeety4
I confirmed that things are okay as of now. Here are the findings.

In case of a saving a new data source

preview is unavailable

user is told that table will be built/rebuilt by HQ

accessing preview link directly shows the new error message

once the table is created by pillows, preview is available and everything works fine

In case of an edit

preview button is still available since table is present

in case there is an error when generating the preview, due to nature of the change made, the user is shown the catch all generic message which seems good enough.

once the table is recreated by pillows, things work fine from there on

In case the table is rebuilt

before the table was even created, the preview button is unavailable

the preview page shows the new message

after the table was created, the preview continues to be available and once the table is rebuilt by pillows, fresh data starts to show up

It was good for me too to be more confident of these cases.
It seems you were thinking of the case when someone clicks on rebuild when the table does not even exist.

Gotta say, I don't love showing psycopg2 exceptions to users. 😑 But okay.

Thanks @mkangia for checking on this. Looks great to me

Gotta say, I don't love showing psycopg2 exceptions to users. 😑 But okay.

Ya, its not great.
It's an additional info for the user here. The relevant message is present above.
I might consider showing this sql exception to devs only, lets see, but in a different PR.

kaapstorm

Looks good.

+1 Ajeet's suggested wording.

mkangia · 2025-01-28T09:55:56Z

Hey @kaapstorm @ajeety4 @Charl1996
requesting for another pass for review please, some additional improvements were requested for a better user experience.

kaapstorm · 2025-01-28T15:22:06Z

corehq/apps/userreports/templates/userreports/edit_data_source.html

+            <span
+              class="hq-help-template"
+              data-title="{% trans_html_attr 'Preview Unavailable' %}"
+              data-content="{% trans_html_attr 'Preview unavailable till the data source table has been created.' %}"


This was a fun read: https://www.merriam-webster.com/grammar/should-you-use-until-or-till-or-til

TLDR:

So there you have it: you will probably wish to avoid ’till, use ’til advisedly, and use both until and till freely. And if you use till in writing and someone tells you that you have made an error, simply take the extra L off the end of the word and poke them in the eye with it.

haha, I admit I wasn't sure how to phrase this and did doubt.

So, I believe this should be?
Preview unavailable until the data source table has been created

946677f
Now I can be at peace, this was troubling me.

kaapstorm · 2025-01-28T15:23:09Z

corehq/apps/userreports/views.py

+            messages.error(
+                self.request,
+                _('Data source table not found!'
+                  '<br/>If you have recently added the data source, please wait for it to be created.'


Gotta say, I don't love showing psycopg2 exceptions to users. 😑 But okay.

sentry-io · 2025-01-30T13:29:51Z

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

‼️ BadSpecError: Problem creating expression: { /a/{domain}/configurable_reports/data_sources/e... View Issue
‼️ BadSpecError: Illegal indicator type: "related_doc", must be one of the following choice: (small_boolean, boole... /a/{domain}/configurable_reports/data_sources/e... View Issue
‼️ BadSpecError: Property expression is required. /a/{domain}/configurable_reports/data_sources/e... View Issue
‼️ BadSpecError: Invalid or missing expression type: [missing] for expression: {'expression': {'type': 'related_do... /a/{domain}/configurable_reports/data_sources/e... View Issue
‼️ BadSpecError: Problem creating expression: { /a/{domain}/configurable_reports/data_sources/e... View Issue

_{Did you find this useful? React with a 👍 or 👎}

mkangia · 2025-02-01T19:53:54Z

To resolve the sentry errors
#35724

report missing table

669a35b

mkangia force-pushed the mk/4091-better-message branch from 1347bfd to 669a35b Compare January 25, 2025 09:44

Merge branch 'master' into mk/4091-better-message

ebfc96b

mkangia marked this pull request as ready for review January 26, 2025 06:15

mkangia requested a review from calellowitz as a code owner January 26, 2025 06:15

mkangia added the product/feature-flag Change will only affect users who have a specific feature flag enabled label Jan 26, 2025

Charl1996 reviewed Jan 27, 2025

View reviewed changes

Charl1996 approved these changes Jan 27, 2025

View reviewed changes

ajeety4 reviewed Jan 27, 2025

View reviewed changes

kaapstorm approved these changes Jan 27, 2025

View reviewed changes

mkangia added 4 commits January 28, 2025 13:42

fix indent

798e270

run prettify-changed.sh

5117caf

de-group buttons to add help block

317f6d4

disable preview button & add help bubble

7b26cbc

mkangia force-pushed the mk/4091-better-message branch from af91e3a to 7b26cbc Compare January 28, 2025 08:44

Merge branch 'master' into mk/4091-better-message

da70531

kaapstorm approved these changes Jan 28, 2025

View reviewed changes

ajeety4 approved these changes Jan 28, 2025

View reviewed changes

mkangia added 2 commits January 29, 2025 15:04

till -> until

946677f

Merge branch 'master' into mk/4091-better-message

c9064d9

mkangia requested review from kaapstorm and ajeety4 January 29, 2025 09:36

kaapstorm approved these changes Jan 29, 2025

View reviewed changes

mkangia merged commit 732a848 into master Jan 29, 2025
14 checks passed

mkangia deleted the mk/4091-better-message branch January 29, 2025 14:05

mkangia mentioned this pull request Feb 1, 2025

Show technical details to superusers only #35723

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better error message for new new data sources #35679

Better error message for new new data sources #35679

mkangia commented Jan 24, 2025 •

edited

Loading

Charl1996 Jan 27, 2025

mkangia Jan 27, 2025

ajeety4 Jan 27, 2025

mkangia Jan 27, 2025

ajeety4 Jan 27, 2025

mkangia Jan 28, 2025

kaapstorm Jan 28, 2025

ajeety4 Jan 28, 2025

mkangia Jan 29, 2025

kaapstorm left a comment

mkangia commented Jan 28, 2025

kaapstorm Jan 28, 2025

mkangia Jan 29, 2025

mkangia Jan 29, 2025

kaapstorm Jan 28, 2025

sentry-io bot commented Jan 30, 2025 •

edited

Loading

mkangia commented Feb 1, 2025

Better error message for new new data sources #35679

Better error message for new new data sources #35679

Conversation

mkangia commented Jan 24, 2025 • edited Loading

Product Description

Technical Summary

Feature Flag

Safety Assurance

Safety story

Automated test coverage

QA Plan

Rollback instructions

Labels & Review

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaapstorm left a comment

Choose a reason for hiding this comment

mkangia commented Jan 28, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sentry-io bot commented Jan 30, 2025 • edited Loading

Suspect Issues

mkangia commented Feb 1, 2025

mkangia commented Jan 24, 2025 •

edited

Loading

sentry-io bot commented Jan 30, 2025 •

edited

Loading