show error if trying unpublished query in alert or dashboard

[alison985]

Original request in mozilla#429 was to port the ability to use unpublished queries in Alerts to from Mozilla's fork to getredash/redash. PR #2660 was rejected because this is not desired behavior in getredash/redash. After additional discussion in mozilla#429 the idea to let the user know that unpublished queries weren't allowed in alerts or dashboards was raised.

This PR:

• Shows a toastr error if a user selects an unpublished query to use in an alert.
• Shows a toastr error if a user selects an unpublished query to use in a dashboard.
• Disallows unpublished queries from being added as widgets to a dashboard. (Previously unpublished queries weren't shown in the default list in the modal but could still be searched for and added.)

[vercel]

This pull request is automatically deployed with Now.

To access deployments, click Details below or on the icon next to each push.

[arikfr]

Letting the user select these queries but then show them an error is a frustrating experience. How about we show them disabled (so they can't be selected) with a text mentioning they need to be published first?

@kocsmy

[alison985]

Thanks @arikfr! I had thought about filtering them out of search results but I thought that would cause bug reports when queries the user knew existed didn't show up as expected. I had also thought about text but thought that would clutter the interface. I love the disabled with text in the dropdown idea. Here is what it looks like for alerts - please let me know you think.

@kocsmy

[alison985]

And here's what it looks like for the Add to Dashboard modal. Because these are A links the visual options are a little different.

[arikfr]

🤔not happy about different styles in how it's presented. @kocsmy do you have any thoughts?

[kocsmy]

I don't think this is bad but maybe it could be improved by

• look more like it's in a disabled state instead of the text-decoration: line-through; Gray text and background.
• the hovered active item should have a different styling.

Shall we just merge this as it is and I'll further improve?

[alison985]

@kocsmy I have been able to restyle the disabled state locally but am not having any luck on the hovered active item.

[jezdez]

@alison985 Could you push your changes for the disabled state here please?

@kocsmy Would you mind elaborating what hovered active items should look like?

[alison985]

I made this update @jezdez. A new screenshot:

@kocsmy Can you elaborate or help on the hovered active item?

[kocsmy]

I'd suggest using 0.5 opacity on the unpublished query and the current hover state should work fine:

[arikfr]

Also worth using the same tag like design for Unpublished text as we use everywhere else.

[alison985]

@kocsmy updated

[alison985]

@kocsmy I wanted to follow up on this one.

[jezdez]

@kocsmy Hey, anything we're missing here to move this along?

[alison985]

@kocsmy & @jezdez - wanted to follow up on this one.

[kocsmy]

Thanks for the nudge @alison985 & @jezdez

• I've checked the design on the modal (for unpublished queries which cannot be added), it all looks good to me.

For the record, this is how it looks:

@arikfr suggested that we use the same tags we are using already.

And as far as I see you implemented that for the alerts:

I think it's a good idea to have the same for the add widget modal on the dashboard.

[alison985]

@kocsmy Updated. Here are all the various states.

Query in Alerts for Published Query

Query in Alerts for Unpublished Query

Add to dashboard before search:

Add to dashboard after search:

Add to dashboard after search with cursor on selection:

Add to dashboard unpublished query:

[kocsmy]

I think @arikfr meant that the (Unpublished) is in the shape of Unpublished tag as well :) I don't think it's a problem that you added the tags as well though, WDYT @arikfr ?

I think the ( ) is not part of the Unpublished tag so I think that can be removed (from the Alerts page too).

I've noticed the tags don't look the best here (shape, color) so that will need some design refinement. I'll address that later.

[kocsmy]

Please also mind the difference between the Unpublished label and tag label design:

[alison985]

@kocsmy I removed the () and updated the styles.

[jezdez]

@kocsmy Hey, following up on this, do you know if there is anything missing in this PR?

[kocsmy]

Hey @jezdez thanks for pinging. @alison985 when I check this PR there's a misalignment (I can't see this on your screenshot though). Can you check and fix if possible, please?

[emtwo]

Closing this PR and will continue discussion/review here: #3347