diff --git a/docs/users-guide/05-visualizing-results.md b/docs/users-guide/05-visualizing-results.md index 9ee0f76d726fc..e2cee3040dfa0 100644 --- a/docs/users-guide/05-visualizing-results.md +++ b/docs/users-guide/05-visualizing-results.md @@ -1,17 +1,17 @@ -## Visualizing results +# Visualizing results -While tables are useful for looking up information or finding specific numbers, it's usually easier to see trends and make sense of data overall using charts. +While tables are useful for looking up information or finding specific numbers, it's usually easier to see trends and make sense of data using charts. In Metabase, an answer to a question can be visualized in a number of ways: -- [Number](#numbers) +- [Numbers](#numbers) - [Trend](#trends) - [Progress bar](#progress-bars) - [Gauge](#gauges) - [Table](#tables) - [Line chart](#line-bar-and-area-charts) - [Bar chart](#line-bar-and-area-charts) -- [Line + bar chart](#line-plus-bar-charts) +- [Combo chart](#line-plus-bar-charts) - [Row chart](#row-charts) - [Area chart](#line-bar-and-area-charts) - [Scatterplot or bubble chart](#scatterplots-and-bubble-charts) @@ -19,27 +19,31 @@ In Metabase, an answer to a question can be visualized in a number of ways: - [Funnel](#funnel) - [Map](#maps) -To change how the answer to your question is displayed, click on the Visualization button in the bottom-right of the screen to open the visualization selection sidebar. +To change how the answer to your question is displayed, click on the **Visualization** button in the bottom-right of the screen to open the visualization sidebar. ![Visualization options](images/VisualizeChoices.png) -If a particular visualization doesn’t really make sense for your answer, that option will appear grayed-out in the sidebar. You can still select a grayed-out option, though you might need to open the chart options to make your selection work with your data. +If a particular visualization doesn’t really make sense for your answer, that option will appear grayed out in the sidebar. You can still select a grayed-out option, though you might need to open the chart options to make your selection work with your data. -Once a question is answered, you can save or download the answer, or add it to a dashboard or Pulse. +Once a question returns results, you can save the question, download the results, or add the question to a [dashboard](07-dashboards.md) or [pulse](10-pulses.md). -### Visualization types and options +## Visualization types and options -Each visualization type has its own advanced options you can tweak. Just click the Settings button next to the Visualization button to see all your options. The options panel also automatically opens up whenever you pick a new visualization type. Here's an overview of what you can do: +Each visualization type has its own advanced options. Click the **Settings** button next to the Visualization button to see your options. The options panel also automatically opens up whenever you pick a new visualization type. Here's an overview of what you can do: -#### Numbers +### Numbers -This option is for displaying a single number, nice and big. The options for numbers include adding character prefixes or suffixes to it (so you can do things like put a currency symbol in front or a percent at the end), setting the number of decimal places you want to include, and multiplying your result by a number (like if you want to multiply a decimal by 100 to make it look like a percent). If you want to _divide_ by a number, then just multiply it by a decimal (e.g, if your result is `100`, but you want it to display as `1`, simply multiply it by 0.01). +The **Numbers** option is for displaying a single number, nice and big. The options for numbers include: + +- **Adding character prefixes or suffixes** to it (so you can do things like put a currency symbol in front or a percent at the end), +- **Setting the number of decimal places** you want to include, and +- **Multiplying your result by a number** (like if you want to multiply a decimal by 100 to make it look like a percent). If you want to _divide_ by a number, then just multiply it by a decimal (e.g, if your result is `100`, but you want it to display as `1`, simply multiply it by 0.01). ![Number](images/visualizations/number.png) -#### Trends +### Trends -The Trend visualization is great for displaying how a single number has changed over time. To use this visualization, you'll need to have a single number grouped by a Time field, like the Count of Orders by Created At. The Trend will show you the value of the number during the most recent period, and below that you'll see how much the number has increased or decreased compared to its value in the period before that. The period is determined by your group-by field: if you're grouping by Day, the Trend will show you the most recent day compared to the day before that. +The **Trend** visualization is great for displaying how a single number has changed over time. To use this visualization, you'll need to have a single number grouped by a Time field, like the Count of Orders by Created At. The Trend will show you the value of the number during the most recent period, as well as how much the number has increased or decreased compared to its value in the previous period. The period is determined by your group-by field; if you're grouping by Day, the Trend will show you the most recent day compared to the day before that. ![Trend](images/visualizations/trend.png) @@ -47,15 +51,15 @@ By default, Trends will display increases as green (i.e. "good") and decreases a ![Trend settings](images/visualizations/trend-settings.png) -#### Progress bars +### Progress bars -Progress bars are for comparing a single number to a goal value that you set. Open up the settings for your progress bar to choose a value for your goal, and Metabase will show you how far away your question's current result is from the goal. +**Progress bars** are for comparing a single number to a goal value that you set. Open up the settings for your progress bar to choose a value for your goal, and Metabase will show you how far away your question's current result is from the goal. ![Progress bar](images/visualizations/progress.png) -#### Gauges +### Gauges -Ah, gauges: you either love 'em or you hate 'em. …Or you feel "meh" about them, I guess. Whatever the case, gauges allow you to show a single number and where its value falls within a set of colored ranges that you can specify. By default, when you choose the Gauge visualization, Metabase will create red, yellow, and green ranges for you. +Ah, **gauges**: you either love 'em or you hate 'em. …Or you feel "meh" about them, I guess. Whatever the case, gauges allow you to show a single number in the context of a set of colored ranges that you can specify. By default, when you choose the Gauge visualization, Metabase will create red, yellow, and green ranges for you. ![Gauge](images/visualizations/gauge.png) @@ -63,51 +67,51 @@ Open up the visualization settings to define your own ranges, choose colors for ![Gauge settings](images/visualizations/gauge-settings.png) -#### Tables +### Tables -The Table option is good for looking at tabular data (duh), or for lists of things like users or orders. The visualization options for tables allow you to add, hide, or rearrange fields in the table you're looking at, as well as modify their formatting. +The **Table** option is good for looking at tabular data (duh), or for lists of things like users or orders. The visualization options for tables allow you to add, hide, or rearrange fields in the table you're looking at, as well as modify their formatting. -##### Rearranging, adding, and removing columns +#### Rearranging, adding, and removing columns ![Additional fields](images/visualizations/add-fields.png) Open up the settings for your table and you'll see the Columns tab, which displays all the columns currently being shown in the table. Below that you'll see a list of more columns from linked tables, if any, that you can add to the current table view. -To hide a column, click the X icon on it; that'll send it down to the "More columns" area in case you want to bring it back. To add a linked column, just click the + icon on it, which will bring it to the "Visible columns" section. Click and drag any of the columns listed there to rearrange the order in which they appear. Another super easy way to rearrange columns without having to open up the visualization settings is to simply click and drag on a column's heading to move it where you'd like it to go. +To hide a column, click the X icon on it; that'll send it down to the **More columns** area in case you want to bring it back. To add a linked column, just click the + icon on it, which will bring it to the **Visible columns** section. Click and drag any of the columns listed there to rearrange the order in which they appear. Another super easy way to rearrange columns without having to open up the visualization settings is to simply click and drag on a column's heading to move it where you'd like it to go. -**Note:** changing these options doesn't change the actual table itself; it just creates a custom view of it that you can save as a "question" in Metabase and refer back to later, share with others, or add to a dashboard. +> Changing these options doesn't change the actual table itself; these changes create a custom view of the table that you can save as a **question** in Metabase and refer back to later, share with others, or add to a [dashboard](08-dashboards.md). -##### Column formatting options +#### Column formatting options To format the display of any column in a table, click on the column heading and choose the `Formatting` option (you can also get there by clicking on the gear on any column when in the `Columns` tab of the visualization settings). ![Column formatting](images/visualizations/column-header-formatting.png) -The options you see will be different depending on the type of column you're viewing: +The options you see will differ depending on the type of column you're viewing: **Dates** -- `Date style` gives you a bunch of different choices for how to display the date. -- `Abbreviate names of days and months`, when turned on, will turn things like `January` to `Jan`, and `Monday` to `Mon`. -- `Show the time` lets you decide whether or not to display the time, and if so, how. You can include hours and minutes, and additionally seconds and milliseconds. +- **Date style** gives you a bunch of different choices for how to display the date. +- **Abbreviate names of days and months**, when turned on, will turn things like `January` to `Jan`, and `Monday` to `Mon`. +- **Show the time** lets you decide whether or not to display the time, and if so, how. You can include hours and minutes, and additionally seconds and milliseconds. **Numbers** -- `Show a mini bar chart` will display a small horizontal bar next to each number in this column to show its size relative to the other values in the column. -- `Style` lets you choose to display the number as a plain number, a percent, in scientific notation, or as a currency. -- `Separator style` gives you various options for how commas and periods are used to separate the number. -- `Minimum number of decimal places` forces the number to be displayed with exactly this many decimal places. -- `Multiply by a number` multiplies each number in this column by whatever you type here. Just don't type an emoji here; it almost always causes a temporal vortex to manifest. -- `Add a prefix/suffix` lets you put a symbol, word, or whatever before or after each cell's value. +- **Show a mini bar chart** will display a small horizontal bar next to each number in this column to show its size relative to the other values in the column. +- **Style** lets you choose to display the number as a plain number, a percent, in scientific notation, or as a currency. +- **Separator style** gives you various options for how commas and periods are used to separate the number. +- **Minimum number of decimal places** forces the number to be displayed with exactly this many decimal places. +- **Multiply by a number** multiplies each number in this column by whatever you type here. Just don't type an emoji here; it almost always causes a temporal vortex to manifest. +- **Add a prefix/suffix** lets you put a symbol, word, or whatever before or after each cell's value. **Currency** Currency columns have all the same options as numbers, plus the following: -- `Unit of Currency` lets you change the unit of currency from whatever the system default is. -- `Currency label style` allows you to switch between displaying the currency label as a symbol, a code like (USD), or the full name of the currency. -- `Where to display the unit of currency` lets you toggle between showing the currency label in the column heading or in every cell in the column. +- **Unit of Currency** lets you change the unit of currency from whatever the system default is. +- **Currency label style** allows you to switch between displaying the currency label as a symbol, a code like (USD), or the full name of the currency. +- **Where to display the unit of currency** lets you toggle between showing the currency label in the column heading or in every cell in the column. -##### Formatting data in charts +#### Formatting data in charts While we're talking about formatting, we thought you should also know that you can access formatting options for the columns used in a chart. Just open the visualization settings and select the `Data` tab: @@ -117,65 +121,66 @@ Then click on the gear icon next to the column that you want to format. Dates, n ![Chart formatting options](images/visualizations/chart-formatting-options.png) -##### Conditional table formatting +#### Conditional table formatting -Sometimes is helpful to highlight certain rows or columns in your tables when they meet a specific condition. You can set up conditional formatting rules by going to the visualization settings while looking at any table, then clicking on the `Formatting` tab. +Sometimes is helpful to highlight certain rows or columns in your tables when they meet a specific condition. You can set up conditional formatting rules by going to the visualization settings while looking at any table, then clicking on the **Conditional Formatting** tab. ![Conditional formatting](images/visualizations/conditional-formatting.png) When you add a new rule, you'll first need to pick which column(s) should be affected. Your columns can be formatted in one of two ways: -1. **Single color:** pick this if you want to highlight cells in the column if they're greater, less than, or equal to a specific number, or if they match or contain a certain word or phrase. You can optionally highlight the whole row of a cell that matches the condition you pick so that it's easier to spot as you scroll down your table. -2. **Color range:** choose this option if you want to tint all the cells in the column from smallest to largest or vice a versa. This option is only available for numeric columns. +- **Single color**. Pick single color if you want to highlight cells in the column if they're greater, less than, or equal to a specific number, or if they match or contain a certain word or phrase. You can optionally highlight the whole row of a cell that matches the condition you pick so that it's easier to spot as you scroll down your table. +- **Color range**. Choose color range if you want to tint all the cells in the column from smallest to largest or vice a versa. This option is only available for numeric columns. You can set as many rules on a table as you want. If two or more rules disagree with each other, the rule that's on the top of your list of rules will win. You can click and drag your rules to reorder them, and click on a rule to edit it. -##### Pivoted tables +#### Pivoted tables -If your table is a result that contains one numeric column and two grouping columns, Metabase will also automatically "pivot" your table, like in the example below. What this does is it takes one of your columns and rotates it 90 degrees ("pivots" it) so that each of its values becomes a column heading. If you open up the visualization settings by clicking the gear icon, you can choose which column to pivot in case Metabase got it wrong; or you can also turn the pivoting behavior off entirely. +If your table is a result that contains one numeric column and two grouping columns, Metabase will also automatically "pivot" your table, like in the example below. Pivoting takes one of your columns and rotates it 90 degrees ("pivots" it) so that each of its values becomes a column heading. If you open up the visualization settings by clicking the gear icon, you can choose which column to pivot in case Metabase got it wrong; or you can also turn the pivoting behavior off entirely. ![Pivot table](images/visualizations/pivot.png) -#### Line, bar, and area charts +### Line, bar, and area charts -Line charts are best for displaying the trend of a number over time, especially when you have lots of x-axis values. Bar charts are great for displaying a number grouped by a category (e.g., the number of users you have by country), and they can also be useful for showing a number over time if you have a smaller number of x-axis values (like orders per month this year). +**Line charts** are best for displaying the trend of a number over time, especially when you have lots of x-axis values. Bar charts are great for displaying a number grouped by a category (e.g., the number of users you have by country). Bar charts can also be useful for showing a number over time if you have a smaller number of x-axis values (like orders per month this year). ![Bar chart](images/visualizations/bar.png) -Area charts are useful when comparing the proportions of two metrics over time. Both bar and area charts can be stacked. +**Area charts** are useful when comparing the proportions of two metrics over time. Both bar and area charts can be stacked. ![Stacked area chart](images/visualizations/area.png) **Trend lines** -Another useful option for line, area, bar, and scatter charts is trend lines. If you have a question where you're grouping by a time field, open up the visualization settings and turn the `Show trend line` toggle on to display a trend line. Metabase will choose the best type of line to fit to the trend of your series. This will even work if you have multiple metrics selected in your summary. But it won't work if you have any groupings beyond the one time field. + +**Trend lines** are another useful option for line, area, bar, and scatter charts. If you have a question where you're grouping by a time field, open up the visualization settings and turn the `Show trend line` toggle on to display a trend line. Metabase will choose the best type of line to fit to the trend of your series. Trend lines will even work if you have multiple metrics selected in your summary. But trend lines won't work if you have any groupings beyond the one time field. ![Trend lines](images/visualizations/trend-lines.png) -#### Line + Bar charts +### Combo charts -Also called Combo Charts, the Line + Bar chart lets you combine bars and lines (or areas) on the same chart. +Combo charts let you combine bars and lines (or areas) on the same chart. ![Line + bar](images/visualizations/combo-chart.png) -Metabase will pick one of your series to display as a line, and another to display as a bar by default. Open up the visualization settings to change which series are lines, bars, or areas, and to change other per-series settings like colors. Click the down arrow icon on the right of a series to see additional options: +Metabase will pick one of your series to display as a line, and another to display as a bar by default. Open up the visualization settings to change which series are lines, bars, or areas, as well as to change per-series settings like colors. Click the down arrow icon on the right of a series to see additional options: ![Line + bar](images/visualizations/combo-chart-settings.png) -To use a Line + Bar chart, you'll either need to have two or more metrics selected in the Summarize By section of your question, with one or two grouping columns, like this… +To use a Combo chart you'll either need to have two or more metrics selected in the Summarize By section of your question, with one or two grouping columns, like this: ![Data for Line + Bar chart](images/visualizations/combo-chart-data-1.png) -…or you'll need a question with a single metric and with two grouping columns, like this: +Or you'll need a question with a single metric and two grouping columns, like this: ![Data for Line + Bar chart](images/visualizations/combo-chart-data-2.png) -#### Row charts +### Row charts -If you're trying to group a number by a column that has a lot of possible values, like a Vendor or Product Title field, try visualizing it as a row chart. Metabase will show you the bars in descending order of size, with a final bar at the bottom for items that didn't fit. +If you're trying to group a number by a column that has a lot of possible values, like a Vendor or Product Title field, try visualizing it as a **row chart**. Metabase will show you the bars in descending order of size, with a final bar at the bottom for items that didn't fit. ![Row chart](images/visualizations/row.png) -##### Histograms +#### Histograms If you have a bar chart like Count of Users by Age, where the x-axis is a number, you'll get a special kind of chart called a **histogram**, where each bar represents a range of values (called a "bin"). Note that Metabase will automatically bin your results any time you use a number as a grouping, even if you aren't viewing a bar chart. Questions that use latitude and longitude will also get binned automatically. @@ -185,7 +190,7 @@ By default, Metabase will automatically choose a good way to bin your results. B ![Binning options](images/notebook/histogram-bins.png) -##### Options for line, bar, and area charts +#### Options for line, bar, and area charts These three charting types have very similar options, which are broken up into the following: @@ -195,69 +200,69 @@ Here's where you can choose the columns you want to plot on your x and y axes. T **Display** -There's quite a bit you can do in this tab: +There's quite a bit you can do in this tab, but the options available will depend on the data in your chart. -- Set the colors and labels for the series on your chart. -- Change the style of your lines for Line and Area charts, and choose whether or not to display dots on them. -- Use the "Replace missing values with…" setting to change how your chart deals with missing values. You can use linear interpolation, or display those points as zero or as nothing. -- Add a goal line. This can be used in conjunction with [alerts](15-alerts.md) to send an email or a Slack message when your metric cross this line. -- If you're looking at a time series chart, you can turn on a trend line to show where things are heading. -- Show values on data points. The default setting will try and fit as many values on your chart as will fit nicely, but you can also force Metabase to show the values for each and every data point, which it will do begrudgingly. +- **Set the colors and labels** for the series on your chart. +- **Change the style of your lines** for Line and Area charts, and choose whether to display dots on the lines. +- **Specify how to handle missing values**. Use the "Replace missing values with…" setting to change how your chart deals with missing values. You can use linear interpolation, or display those points as zero or as nothing. +- **Add a goal line**. Goal lines can be used in conjunction with [alerts](15-alerts.md) to send an email or a Slack message when your metric cross this line. +- **Add a trend line**. If you're looking at a time series chart, you can turn on a trend line to show where things are heading. +- **Show values on data points**. The default setting will try and fit as many values on your chart as will fit nicely, but you can also force Metabase to show the values for each and every data point, which it will do begrudgingly. Showing values also works with multi-series charts, but be aware that the more data points you add, the more crowded with values the charts will become. **Axes** -There are three main things you can do here: +There are three main ways to configure axes: -- Change the scale for your axes. If you're looking at a time series chart, your x-axis can use a time series scale or an ordinal one. When using "Timeseries", it will always be displayed in ascending order, so oldest to newest, while "Ordinal" will display in the order the data is returned. Your y-axis can use a linear, power, or logarithmic scale. -- Hide or show the tick marks on your axes. You can also choose to rotate the tick marks on the x-axis to help them fit better. -- Edit the range of your y-axis. Metabase sets an automatic range by default, but you can toggle that off and input a custom minimum and maximum value for the y-axis if you'd like. +- **Change the scale for your axes**. If you're looking at a time series chart, your x-axis can use a time series scale or an ordinal one. When using "Timeseries", it will always be displayed in ascending order, so oldest to newest, while "Ordinal" will display in the order the data is returned. Your y-axis can use a linear, power, or logarithmic scale. +- **Hide or show the tick marks on your axes**. You can also choose to rotate the tick marks on the x-axis to help them fit better. +- **Edit the range of your y-axis**. Metabase sets an automatic range by default, but you can toggle that off and input a custom minimum and maximum value for the y-axis if you'd like. **Labels** -Here's where you can choose to hide the label for your x- or y-axis. You can also customize the text for your axis labels here. +Here's where you can choose to hide the **label** for your x- or y-axis. You can also customize the text for your axes labels here. -#### Scatterplots and bubble charts +### Scatterplots and bubble charts -Scatterplots are useful for visualizing the correlation between two variables, like comparing the age of your users vs. how many dollars they've spent on your products. To use a scatterplot, you'll need to ask a question that results in two numeric columns, like `Count of Orders grouped by Customer Age`. Alternatively, you can use a raw data table and select the two numeric fields you want to use in the chart options. +**Scatterplots** are useful for visualizing the correlation between two variables, like comparing the age of your users vs. how many dollars they've spent on your products. To use a scatterplot, you'll need to ask a question that results in two numeric columns, like `Count of Orders grouped by Customer Age`. Alternatively, you can use a raw data table and select the two numeric fields you want to use in the chart options. -If you have a third numeric field, you can also create a bubble chart. Select the Scatter visualization, then open up the chart settings and select a field in the `bubble size` dropdown. This field will be used to determine the size of each bubble on your chart. For example, you could use a field that contains the total dollar amount for each x-y pair — i.e., larger bubbles for larger total dollar amounts spent on orders. +If you have a third numeric field, you can also create a **bubble chart**. Select the Scatter visualization, then open up the chart settings and select a field in the **bubble size** dropdown. This field will be used to determine the size of each bubble on your chart. For example, you could use a field that contains the total dollar amount for each x-y pair — i.e. larger bubbles for larger total dollar amounts spent on orders. Scatterplots and bubble charts also have similar chart options as line, bar, and area charts, including the option to display trend or goal lines. ![Scatter](images/visualizations/scatter.png) -#### Pie or donut charts +### Pie or donut charts -A pie or donut chart can be used when breaking out a metric by a single dimension, especially when the number of possible breakouts is small, like users by gender. If you have more than a few breakouts, like users per country, it's usually better to use a bar chart so that your users can more easily compare the relative sizes of each bar. +A **pie or donut chart** can be used when breaking out a metric by a single dimension, especially when the number of possible breakouts is small, like users by gender. If you have more than a few breakouts, like users per country, it's usually better to use a bar chart so that your users can more easily compare the relative sizes of each bar. -The options for pie charts let you choose which field to use as your measurement, and which one to use for the dimension (i.e., the pie slices). You can also customize the color of each piece slice, the pie chart's legend, whether or not to show each slice's percent of the whole in the legend, and the minimum size a slice needs to be in order for it to be displayed. +The options for pie charts let you choose which field to use as your measurement, and which one to use for the dimension (i.e. the pie slices). You can also customize the color of each slice, the pie chart's legend, whether or not to show each slice's percent of the whole in the legend, and the minimum size a slice needs to be in order for Metabase to display it. ![Donut](images/visualizations/donut.png) -#### Funnel +### Funnel -Funnels are commonly used in e-commerce or sales to visualize how many customers are present within each step of a checkout flow or sales cycle. At their most general, funnels show you values broken out by steps, and the percent decrease between each successive step. To create a funnel in Metabase, you'll need to have a table with at least two columns: one column that contains the metric you're interested in, and another that contains the funnel's steps. +**Funnels** are commonly used in e-commerce or sales to visualize how many customers are present within each step of a checkout flow or sales cycle. At their most general, funnels show you values broken out by steps, and the percent decrease between each successive step. To create a funnel in Metabase, you'll need to have a table with at least two columns: one column that contains the metric you're interested in, and another that contains the funnel's steps. For example, I might have an Opportunities table, and I could create a question that gives me the number of sales leads broken out by a field that contains stages such as `Prospecting`, `Qualification`, `Proposal`, `Negotiation`, and `Closed`. In this example, the percentages shown along the x-axis tell you what percent of the total starting opportunities are still present at each subsequent step; so 18.89% of our total opportunities have made it all the way to being closed deals. The number below each percent is the actual value of the count at that step — in our example, the actual number of opportunities that are currently at each step. Together, these numbers help you figure out where you're losing your customers or users. ![Funnel](images/visualizations/funnel.png) -#### Maps +### Maps -When you select the Map visualization setting, Metabase will automatically try and pick the best kind of map to use based on the table or result set you're currently looking at. Here are the maps that Metabase uses: +When you select the **Map** visualization setting, Metabase will automatically try and pick the best kind of map to use based on the table or result set. Here are the maps that Metabase uses: -- **United States Map** — Creating a map of the United States from your data requires your results to contain a column that contains names of states or two-letter state codes. This lets you do things like visualize the count of your users broken out by state, with darker states representing more users. -- **World Map** — To visualize your results in the format of a map of the world broken out by country, your result must contain a column with two-letter country codes. (E.g., count of users by country.) +- **United States Map**. Creating a map of the United States from your data requires your results to contain a column that contains names of states or two-letter state codes. This lets you do things like visualize the count of your users broken out by state, with darker states representing more users. +- **World Map**. To visualize your results in the format of a map of the world broken out by country, your result must contain a column with two-letter country codes. (E.g., count of users by country.) ![Region map](images/visualizations/map.png) -- **Pin Map** — If your table contains a latitude and longitude field, Metabase will try to display it as a pin map of the world. This will put one pin on the map for each row in your table, based on the latitude and longitude fields. You can try this with the Sample Dataset that's included in Metabase: start a new question and select the People table, use `raw data` for your view, and choose the Map option for your visualization. you'll see a map of the world, with each dot representing the latitude and longitude coordinates of a single person from the People table. +- **Pin Map**. If your results contains a latitude and longitude field, Metabase will try to display the results as a pin map of the world. Metabase will put one pin on the map for each row in your table, based on the latitude and longitude fields. You can try this with the Sample Dataset that's included in Metabase: start a new question and select the People table, use `raw data` for your view, and choose the Map option for your visualization. You'll see a map of the world, with each dot representing the latitude and longitude coordinates of a single person from the People table. ![Pin map](images/visualizations/pin-map.png) -When you open up the Map options, you can manually switch between a region map (i.e., United States or world) and a pin map. If you're using a region map, you can also choose which field to use as the measurement, and which to use as the region (i.e. State or Country). +When you open up the Map options, you can manually switch between a region map (e.g., United States) and a pin map. If you're using a region map, you can also choose which field to use as the measurement, and which field to use as the region (e.g., State or Country). -Metabase also allows administrators to add custom region maps via GeoJSON files through the Metabase Admin Panel. +Metabase also allows administrators to add custom region maps via GeoJSON files through the Metabase **Admin Panel**. --- diff --git a/docs/users-guide/09-multi-series-charting.md b/docs/users-guide/09-multi-series-charting.md index 785a52f5171ac..e42a444e3d1d9 100644 --- a/docs/users-guide/09-multi-series-charting.md +++ b/docs/users-guide/09-multi-series-charting.md @@ -7,73 +7,88 @@ Data in isolation is rarely all that useful. One of the best ways to add context - New users per day vs. returning users per day. - Orders per day from a few different product lines. -### In Metabase there are two main ways to get data side-by-side. +### Displaying data side by side -1. Combining two existing saved questions that share a common dimension (like time) on a dashboard +There are two main ways to visualize data side by side: -e.g. Let me see revenue over time and cost over time together. +1. [**Ask a question that involves multiple dimensions**](#ask-a-question-that-involves-multiple-dimensions) with the Simple or Custom query builders (or in SQL, if you’re fancy). Example: the count of users by region over time. -2. Asking a question that involves multiple dimensions in the query builder (or in SQL if you’re fancy). +2. [**Combine two saved questions**](#combining-two-saved-questions) that share a common dimension (like time) on a dashboard. For example, you could look at revenue over time and costs over time together. -e.g. The count of users by region over time. +### Ask a question that involves multiple dimensions -### Combining two existing saved questions +If you’re creating a new question, you can view the results as a multi-series visualization by summarizing your data and grouping it into two groups. -If you already have two or more saved questions you’d like to compare, and they share a dimension, they can be combined on any dashboard. Here’s how: +As an example, we might want to see which website or service is referring the most people to our website. In the **Sample Dataset** that ships with Metabase, you would group by the `Source` and `Created At` columns of the **People** table. + +To create the multi-series chart, select the **People** table, click on the **Summarize** button in the upper right, then add `Source` and `Created At` as groupings (the `count of rows` metric that we want is selected by default). Be sure to click the plus button to the right of your selection, so Metabase knows to add the grouping; otherwise, Metabase will switch to that grouping. [Learn more about asking questions](04-asking-questions.md). + +Metabase will automatically display a multi-series line chart visualization of how each referrer has performed for us. + +![multi-series in the query builder](images/multi-series-charts/multi-series_query_builder.png) + +You can also create a multi-series chart by composing a custom question in the notebook editor. All you need to do is summarize your data (e.g., count the rows) and group that data into multiple groups (e.g. `Created At` by month and Product Category). -1. Add a question with a dimension like time or a category to a dashboard. In practice, these will usually be line charts or bar charts. +![Composing a multi-series question in the notebook editor](images/multi-series-charts/notebook_editor_multi-series.png) -2. While in edit mode on the dashboard, hovering on the card will show a “Edit data” button. Click this button to start adding series that you want to compare to the first series.. +Note: you won’t be able to add another saved question to multi-series visualizations made in this fashion. Metabase can visualize up to 100 distinct values of a dimension at once, so if you're selecting a field that contains a lot of values, you might need to filter the values. -![multiseriestrigger](images/MultiSeriesTrigger.png) -3. In the Edit Data modal you’ll see the original question, and on the right you’ll see a list of compatible questions you can choose from. Check the box next to any questions you’d like to see side-by-side with the original, and Metabase will add it to the same chart. +### Combining two saved questions -![multiseriesmodal1](images/MultiSeriesModal1.png) +If you already have two or more saved questions you’d like to compare, and they share a dimension, they can be combined on any dashboard. Here’s how: + +1. Add a question with a dimension like time or category to a dashboard. In practice, these will usually be line charts or bar charts. + +2. While in edit mode on the dashboard, hovering over a card will display some editing options in the upper right of the question, including an option to **add a line**, as well as a **gear** icon. Click on the add a line option (the **+** with a line and the word "Add" next to it). -The X and Y axis will automatically update if necessary and Metabase will create a legend using the existing card titles to help you understand which question maps to which series on the chart. Repeat this process as many times as you need. +![add multi-series](images/multi-series-charts/add_series.png) -To remove a series either uncheck the box, or click the x next to the title in the legend above the chart. +3. In the Edit Data modal, you’ll see the original question on the left, with a list of compatible questions you can choose from on the right. Search question(s) to add, and check the box next to each question you’d like to see alongside with the original. Metabase will add the question(s) to the same chart. -![multiseriesmodal2](images/MultiSeriesModal2.png) +![multi-series edit modal](images/multi-series-charts/edit_modal.png) -Once you have your chart looking how you’d like, hit done and your changes will be shown on the card in the dashboard. Depending on how dense your data is, at this point you might want to consider enlarging your chart to make sure the data is legible. +If necessary, the X and Y axes will automatically update. Metabase will create a legend using the existing card titles to help you understand which question maps to which series on the chart. Repeat this process as many times as you need. -![multiseriefinished](images/MultiSeriesFinished.png) +![Edit modal with multi-series](images/multi-series-charts/edit_modal_multi-series.png) -#### A quick note about SQL based questions. +To remove a series, simply uncheck its box. -Metabase has less information about SQL based questions, so we cannot guarantee if they can be added reliably. You'll see a little warning sign next to SQL questions to indicate this and when you try adding them just be aware it may not work. +Once you have your chart looking how you’d like, hit done. Metabase will show your changes on the card in the dashboard. Depending on how dense your data is, at this point you might want to consider enlarging your chart to make sure the data is legible. + +> **SQL questions may not work**. Metabase has less information about SQL-based questions, so we cannot guarantee they can be added reliably. You'll see a little warning sign next to SQL questions to indicate this uncertainty, so be aware that adding these kinds of questions may not work. ### Combining Number charts -If you need to compare single numbers and get a sense of how they differ, Metabase also lets you turn multiple Number charts into a bar chart. To do this, follow the same process outlined above. While editing a dashboard, click “edit data” on the Number chart of your choice and then select the other saved question(s) you’d like to see represented on the bar chart. (At Metabase, we use this to create simple funnel visualizations.) +If you need to compare single numbers to get a sense of how they differ, Metabase can turn multiple number charts into a bar chart. -### Creating a multi-series visualization in the query builder. +As above, while editing a dashboard, hover over a number chart of your choice, and click on the icon with the **+** and bar chart icon to add a saved question. -If you’re creating a new question you can also view the result as a multi-series visualization. To do this you’ll need to summarize and add two groupings to your question. +![Add bar chart](images/multi-series-charts/add_bar_chart.png) -As an example, we might want to see which website or service is referring the most people to our website. (In the sample dataset that ships with Metabase this would involve using the `Source` and `Created At` columns of the `People` table.) +Use the search bar to find other saved question(s) that you’d like to see represented on the bar chart, and click the checkbox to add them to your chart. In this case, we added **Widget orders** to compare them to **Gadget orders**. -To do this we’d click the Summarize button, and then add `Source` and `Created At` as groupings (the `count of rows` metric that we want is already selected by default). [Learn more about asking questions](04-asking-questions.md). +![From numbers to bar chart](images/multi-series-charts/numbers_to_bar_chart.png) -Metabase will automatically display a multi-series line chart visualization of how each referrer has performed for us. +### Multi-series charts, values, and legibility + +When displaying multiple series, it’s important to keep legibility in mind. Combining many series can sometimes decrease the communication value of the data. -![multiseriesquerybuilder](images/MultiSeriesQueryBuilder.png) +Metabase allows you to add values to multi-series charts, but go easy on this feature, especially on charts with lots of data points. Adding values to multiple series, each with many data points, can make charts more difficult to read. -It’s worth noting that at this time you won’t be able to add another saved question to multi-series visualizations made in this fashion. Metabase can only visualize up to 20 values of a dimension at once, so you may need to filter the values if you're selecting a field that contains a lot of values like "State." +From the **Visualization -> Display** options, you can toggle the option: **Show values on data points**. Metabase will do its best to fit as many values as can fit nicely. You can also force Metabase to (begrudgingly) show values for all data points, by setting the **Values to show** to **All**. -### Other multiple series tips +![add values to multi-series chart](images/multi-series-charts/add_values.gif) -- When displaying multiple series it’s important to keep legibility in mind. Combining many series can sometimes decrease the communication value of the data. +You can also toggle values for each individual series. If you have three series, for example, you can show values on one, two, or all three series. In the **Display** tab, click on the down arrow to the right of a series to expand its details, and toggle **Show values for this series** to show or hide its values. You can also toggle values for the whole chart, then selectively hide values for individual series until you have your chart looking just right. -### To recap: +Additionally, there is an option to configure the formatting of the values: -- Existing saved questions can be combined and displayed on dashboards when editing the dashboard. -- Scalars can be combined to create bar charts and simple funnels -- You can produce a multi-series visualization in the query builder by adding two dimensions to your query. +- **Auto**. Metabase selects the appropriate style for you +- **Compact**. Metabase abbreviates values, e.g., 1,000 becomes 1K. +- **Full**. Values are displayed in their natural beauty. -Go forth and start letting your data get to know each other. +Now go forth and start letting your data get to know each other. --- diff --git a/docs/users-guide/custom-questions.md b/docs/users-guide/custom-questions.md index fad2ddfb7719a..f07f47011718c 100644 --- a/docs/users-guide/custom-questions.md +++ b/docs/users-guide/custom-questions.md @@ -86,12 +86,14 @@ After you click on the Join Data button to add a join step, you'll need to pick Next, you'll need to pick the columns you want to join on. This means you pick a column from the first table, and a column from the second table, and the join will stitch rows together where the value from the first column is equal to the value in the second column. A very common example is to join on an ID column in each table, so if you happened to pick a table to join on where there is a foreign key relationship between the tables, Metabase will automatically pick those corresponding ID columns for you. At the end of your join step, there's a `Columns` button you can click to choose which columns you want to include from the joined data. -By default, Metabase will do a left outer join, but you can click on the Venn diagram icon to change this to a different type of join. The options you'll see will differ based on the type of database you're using. Here are what the basic types of joins each do: +By default, Metabase will do a left outer join, but you can click on the Venn diagram icon to select a different type of join. Not all databases support all types of joins, so Metabase will only display the options supported by the database you're using. + +Here are the basic types of joins: - **Left outer join:** select all records from Table A, along with records from Table B that meet the join condition, if any. -- **Right outer join:** select all records from Table B, along with records from Table B that meet the join condition, if any. +- **Right outer join:** select all records from Table B, along with records from Table A that meet the join condition, if any. - **Inner join:** only select the records from Table A and B where the join condition is met. -- **Full outer join:** select all records from both tables, whether or not the join condition is met. This is not available for MySQL or H2, and is only available for some database types, like Spark SQL, SQL Server, and SQLite. +- **Full outer join:** select all records from both tables, whether or not the join condition is met. **A left outer join example:** If Table A is Orders and Table B is Customers, and assuming you do a join where the `customer_id` column in Orders is equal to the `ID` column in Customers, when you do a left outer join your results will be a full list of all your orders, and each order row will also display the columns of the customer who placed that order. Since a single customer can place many orders, a given customer's information might be repeated many times for different order rows. If there isn't a corresponding customer for a given order, the order's information will be shown, but the customer columns will just be blank for that row. diff --git a/docs/users-guide/expressions.md b/docs/users-guide/expressions.md index 5db02368f94f4..7f8e0fe10fe7b 100644 --- a/docs/users-guide/expressions.md +++ b/docs/users-guide/expressions.md @@ -94,21 +94,18 @@ This would return rows where `Created At` is between January 1, 2020 and March 3 Certain database types don't support some of the above functions: -MySQL and SQL Server +**BigQuery**: `abs`, `ceil`, `floor`, `median`, `percentile` and `round` -- median -- percentile +**H2**: `median`, `percentile` and `regexextract` -SQLite +**MySQL**: `median`, `percentile` and `regexextract` -- log -- median -- percentile -- power -- standardDeviation -- sqrt -- variance +**SQL Server**: `median`, `percentile` and `regexextract` -Additionally, **Vertica**, **BigQuery**, and **Presto** only provide _approximate_ results for median and percentile. +**SQLite**: `log`, `median`, `percentile`, `power`, `regexextract`, `standardDeviation`, `sqrt` and `variance` + +**Vertica**: `median` and `percentile` + +Additionally, **Presto** only provides _approximate_ results for `median` and `percentile`. If you're using or maintaining a third-party database driver, please [refer to the wiki](https://github.com/metabase/metabase/wiki/What's-new-in-0.35.0-for-Metabase-driver-authors) to see how your driver might be impacted. diff --git a/docs/users-guide/images/MultiSeriesFinished.png b/docs/users-guide/images/MultiSeriesFinished.png deleted file mode 100644 index 6c3fa315523e9..0000000000000 Binary files a/docs/users-guide/images/MultiSeriesFinished.png and /dev/null differ diff --git a/docs/users-guide/images/MultiSeriesModal1.png b/docs/users-guide/images/MultiSeriesModal1.png deleted file mode 100644 index 2177570242752..0000000000000 Binary files a/docs/users-guide/images/MultiSeriesModal1.png and /dev/null differ diff --git a/docs/users-guide/images/MultiSeriesModal2.png b/docs/users-guide/images/MultiSeriesModal2.png deleted file mode 100644 index 1eebc28efb2ff..0000000000000 Binary files a/docs/users-guide/images/MultiSeriesModal2.png and /dev/null differ diff --git a/docs/users-guide/images/MultiSeriesQueryBuilder.png b/docs/users-guide/images/MultiSeriesQueryBuilder.png deleted file mode 100644 index dc45297e86bea..0000000000000 Binary files a/docs/users-guide/images/MultiSeriesQueryBuilder.png and /dev/null differ diff --git a/docs/users-guide/images/MultiSeriesTrigger.png b/docs/users-guide/images/MultiSeriesTrigger.png deleted file mode 100644 index c3d1a92ab368b..0000000000000 Binary files a/docs/users-guide/images/MultiSeriesTrigger.png and /dev/null differ diff --git a/docs/users-guide/images/SQL-filter-widget.png b/docs/users-guide/images/SQL-filter-widget.png index f4af9e26b7b63..bdbbf255b92fc 100644 Binary files a/docs/users-guide/images/SQL-filter-widget.png and b/docs/users-guide/images/SQL-filter-widget.png differ diff --git a/docs/users-guide/images/SQLInterface.png b/docs/users-guide/images/SQLInterface.png index 57c03b6ab76c6..4116c733e5a81 100644 Binary files a/docs/users-guide/images/SQLInterface.png and b/docs/users-guide/images/SQLInterface.png differ diff --git a/docs/users-guide/images/multi-series-charts/add_bar_chart.png b/docs/users-guide/images/multi-series-charts/add_bar_chart.png new file mode 100644 index 0000000000000..ff9270ec95165 Binary files /dev/null and b/docs/users-guide/images/multi-series-charts/add_bar_chart.png differ diff --git a/docs/users-guide/images/multi-series-charts/add_series.png b/docs/users-guide/images/multi-series-charts/add_series.png new file mode 100644 index 0000000000000..508f596e28f12 Binary files /dev/null and b/docs/users-guide/images/multi-series-charts/add_series.png differ diff --git a/docs/users-guide/images/multi-series-charts/add_values.gif b/docs/users-guide/images/multi-series-charts/add_values.gif new file mode 100644 index 0000000000000..013b02af5941f Binary files /dev/null and b/docs/users-guide/images/multi-series-charts/add_values.gif differ diff --git a/docs/users-guide/images/multi-series-charts/edit_modal.png b/docs/users-guide/images/multi-series-charts/edit_modal.png new file mode 100644 index 0000000000000..2bc8812b99520 Binary files /dev/null and b/docs/users-guide/images/multi-series-charts/edit_modal.png differ diff --git a/docs/users-guide/images/multi-series-charts/edit_modal_multi-series.png b/docs/users-guide/images/multi-series-charts/edit_modal_multi-series.png new file mode 100644 index 0000000000000..261a7206e4737 Binary files /dev/null and b/docs/users-guide/images/multi-series-charts/edit_modal_multi-series.png differ diff --git a/docs/users-guide/images/multi-series-charts/multi-series_query_builder.png b/docs/users-guide/images/multi-series-charts/multi-series_query_builder.png new file mode 100644 index 0000000000000..0bd5ebf3ca733 Binary files /dev/null and b/docs/users-guide/images/multi-series-charts/multi-series_query_builder.png differ diff --git a/docs/users-guide/images/multi-series-charts/notebook_editor_multi-series.png b/docs/users-guide/images/multi-series-charts/notebook_editor_multi-series.png new file mode 100644 index 0000000000000..e337bd7087247 Binary files /dev/null and b/docs/users-guide/images/multi-series-charts/notebook_editor_multi-series.png differ diff --git a/docs/users-guide/images/multi-series-charts/numbers_to_bar_chart.png b/docs/users-guide/images/multi-series-charts/numbers_to_bar_chart.png new file mode 100644 index 0000000000000..276eb29c0582a Binary files /dev/null and b/docs/users-guide/images/multi-series-charts/numbers_to_bar_chart.png differ diff --git a/docs/users-guide/images/sql-snippets/highlight_and_save_as_snippet.gif b/docs/users-guide/images/sql-snippets/highlight_and_save_as_snippet.gif new file mode 100644 index 0000000000000..df2336fd0f840 Binary files /dev/null and b/docs/users-guide/images/sql-snippets/highlight_and_save_as_snippet.gif differ diff --git a/docs/users-guide/images/sql-snippets/snippet_sidebar_and_insertion.gif b/docs/users-guide/images/sql-snippets/snippet_sidebar_and_insertion.gif new file mode 100644 index 0000000000000..7dd64b115e533 Binary files /dev/null and b/docs/users-guide/images/sql-snippets/snippet_sidebar_and_insertion.gif differ diff --git a/docs/users-guide/sql-snippets.md b/docs/users-guide/sql-snippets.md new file mode 100644 index 0000000000000..6d3342938e229 --- /dev/null +++ b/docs/users-guide/sql-snippets.md @@ -0,0 +1,84 @@ +## SQL snippets + +![Highlight and save as snippet](./images/sql-snippets/highlight_and_save_as_snippet.gif) + +**SQL snippets** are reusable bits of SQL or native queries. Anyone with permissions to the [SQL editor](writing-sql.md) can create and edit snippets, which are then available for all SQL authors. + +For example, if you frequently perform queries that involve multiple tables, you can save the SQL code that joins those tables as a snippet so that you (and others in your organization) can reuse that code in multiple questions. + +### How to create a snippet + +Here's a simple query with a join using the **Sample Dataset** included with Metabase. + +``` +SELECT * +FROM orders AS o +LEFT JOIN products AS p +ON o.product_id = p.id +``` + +Let's save everything after FROM as a snippet to reuse in other queries. + +In the **SQL editor**: + +1. **Highlight a section of SQL** that you want to save. In this case, we'll select the following SQL code: +``` +orders AS o +LEFT JOIN products AS p +ON o.product_id = p.id +``` + +2. **Right-click on the highlighted section.** +3. **Select Save as snippet** to create a snippet. A modal will pop up with the SQL statement you highlighted. +4. **Edit, name, and describe your snippet**, then click the save button. + +In this case, we named the snippet "Orders and Products". The snippet will now be available for anyone to use. Here's what the snippet looks like in the SQL editor: + +``` +SELECT * +FROM {{snippet: Orders and Products}}; +``` + +When writing in the SQL editor, you can now start typing `{{snippet:` and Metabase will present autocomplete options for available snippets. + +### Snippet menu + +![Snippet sidebar and insertion](./images/sql-snippets/snippet_sidebar_and_insertion.gif) + +The SQL editor **sidebar** has a **SQL Snippets** menu to list available and archived snippets. + +Click on the snippet icon on the right side of the SQL editor, below the Data Reference book icon and the Variables χ icon. Metabase will slide out a sidebar menu that lists available snippets. + +From the SQL Snippets menu, you can: + +- **Create a snippet.** Click on the `+` in the upper right of the SQL Snippets sidebar to create a new snippet. +- **Preview snippets.** Click on the down arrow to the right of a snippet to see its description and a preview of its SQL code. There's also an option to edit the snippet. +- **Insert a snippet.** Click on a snippet's name to insert it into your query at the cursor's current location. +- [**Edit a snippet.**](#editing-snippets) You can change a snippet's name, description and code. +- [**Archive and unarchive a snippet.**](#archived-snippets) From the Edit modal, you can archive a snippet, which removes the snippet from the snippet menu and autocomplete options in the SQL editor. + +### Editing snippets + +You can **edit** a snippet at any time by selecting the snippet from the SQL Snippets sidebar menu in the SQL editor. Click on the down arrow to the right of the snippet, then click **Edit**. You can change the SQL code, snippet name, and snippet description. + +Editing snippets is a great way to make changes to many questions at once. If, for example, you've saved the SQL code to pull user data from tables X, Y, and Z as the snippet `User Data`, but you need to change how that data is pulled (such as by adding data from another column or table), you can update the SQL code in the snippet, and all questions that use the snippet `User Data` will have the updated code. + +**Editing a snippet's name**. Changing a snippet's name will update the snippet's name in every question that uses that snippet. It won't break any existing questions (the underlying SQL remains unchanged), but be aware that other users may be caught off guard to discover you renamed a snippet they use frequently from "Orders and Products" to "All the things", or whatever. + +**Editing a snippet's SQL.** Here's where we have to remind you that with great power comes great responsibility. There is one major caveat when editing snippets, worthy of a callout: + +> **Caution: if you edit a snippet and include broken code, you will break every question that uses that snippet.** Make sure to test your code before saving it to an existing snippet. + +### Archived snippets + +**Archiving** snippets can help keep dated or less relevant snippets out of the way. When you archive a snippet, the snippet no longer populates in the snippet autocomplete dropdown, and the snippet will no longer show up in the main list of of snippets in the **SQL editor** sidebar. + +Archiving a snippet does not affect any existing queries that use the snippet, so you can safely archive a snippet without impacting any questions. + +You can access an archived snippet from the snippet sidebar menu by clicking on the archived button in the bottom left of the sidebar. + +Although there is no way to delete a snippet, you can archive and unarchive a snippet at any time. + +### Snippet permissions + +Any user who has SQL editor permissions to at least one of your connected databases will be able to view the snippets sidebar, and will be able to create, edit, and archive or unarchive any and all snippets — even snippets intended to be used with databases the user does NOT have SQL editing access to. diff --git a/docs/users-guide/start.md b/docs/users-guide/start.md index 58c72ba91a0bc..31635a258daef 100644 --- a/docs/users-guide/start.md +++ b/docs/users-guide/start.md @@ -27,6 +27,7 @@ - [Referencing your data model while writing SQL](12-data-model-reference.md) - [Creating SQL Templates](13-sql-parameters.md) +- [Using SQL snippets to reuse and share code](sql-snippets.md) - [Getting automatic insights with X-rays](14-x-rays.md) - [Setting and getting alerts](15-alerts.md) diff --git a/docs/users-guide/writing-sql.md b/docs/users-guide/writing-sql.md index 6c910d6d8a723..c45d4671f879c 100644 --- a/docs/users-guide/writing-sql.md +++ b/docs/users-guide/writing-sql.md @@ -24,12 +24,16 @@ Questions asked using SQL can be saved, downloaded, or added to a dashboard just ### Using SQL filters -If you or someone else wrote a SQL query that includes [variables](13-sql-parameters.md), your question might have filter widgets at the top of the screen. These let you modify and filter the SQL query before it's run, changing the results you might get. +If you or someone else wrote a SQL query that includes [variables](13-sql-parameters.md), your question might have filter widgets at the top of the screen. Filter widgets let you modify the SQL query before it's run, changing the results you might get. ![SQL filter](images/SQL-filter-widget.png) Writing SQL queries that use variables or parameters can be very powerful, but it's also a bit more advanced, so that topic has its own page if you'd like to [learn more](13-sql-parameters.md). +### SQL snippets + +You can use [SQL snippets](sql-snippets.md) to save, reuse, and share SQL code across multiple questions that are composed using the SQL editor. + --- ## Next: Creating charts diff --git a/frontend/src/metabase-lib/lib/queries/StructuredQuery.js b/frontend/src/metabase-lib/lib/queries/StructuredQuery.js index 5ef28c49530f0..cc47df08172fc 100644 --- a/frontend/src/metabase-lib/lib/queries/StructuredQuery.js +++ b/frontend/src/metabase-lib/lib/queries/StructuredQuery.js @@ -816,9 +816,14 @@ export default class StructuredQuery extends AtomicQuery { return [].concat(...queries.map(q => q.filters())); } - filterFieldOptionSections(filter?: ?(Filter | FilterWrapper)) { + filterFieldOptionSections( + filter?: ?(Filter | FilterWrapper), + { includeSegments = true } = {}, + ) { const filterDimensionOptions = this.filterDimensionOptions(); - const filterSegmentOptions = this.filterSegmentOptions(filter); + const filterSegmentOptions = includeSegments + ? this.filterSegmentOptions(filter) + : []; return filterDimensionOptions.sections({ extraItems: filterSegmentOptions.map(segment => ({ name: segment.name, diff --git a/frontend/src/metabase-lib/lib/queries/structured/Aggregation.js b/frontend/src/metabase-lib/lib/queries/structured/Aggregation.js index 34226a6776f1b..52009696a9f33 100644 --- a/frontend/src/metabase-lib/lib/queries/structured/Aggregation.js +++ b/frontend/src/metabase-lib/lib/queries/structured/Aggregation.js @@ -6,7 +6,7 @@ import { t } from "ttag"; import { TYPE } from "metabase/lib/types"; -import { isStandard, isMetric, isCustom } from "metabase/lib/query/aggregation"; +import * as AGGREGATION from "metabase/lib/query/aggregation"; import type { Aggregation as AggregationObject } from "metabase/meta/types/Query"; import type StructuredQuery from "../StructuredQuery"; @@ -160,21 +160,21 @@ export default class Aggregation extends MBQLClause { * Returns true if this is a "standard" metric */ isStandard(): boolean { - return isStandard(this); + return AGGREGATION.isStandard(this); } /** * Returns true if this is a metric */ isMetric(): boolean { - return isMetric(this); + return AGGREGATION.isMetric(this); } /** * Returns true if this is custom expression created with the expression editor */ isCustom(): boolean { - return isCustom(this); + return AGGREGATION.isCustom(this); } // STANDARD AGGREGATION diff --git a/frontend/src/metabase/admin/datamodel/containers/MetricForm.jsx b/frontend/src/metabase/admin/datamodel/components/MetricForm.jsx similarity index 56% rename from frontend/src/metabase/admin/datamodel/containers/MetricForm.jsx rename to frontend/src/metabase/admin/datamodel/components/MetricForm.jsx index 63a30e32076da..2d14ce9356d63 100644 --- a/frontend/src/metabase/admin/datamodel/containers/MetricForm.jsx +++ b/frontend/src/metabase/admin/datamodel/components/MetricForm.jsx @@ -1,5 +1,7 @@ import React, { Component } from "react"; import { Link } from "react-router"; +import { reduxForm } from "redux-form"; +import cx from "classnames"; import FormLabel from "../components/FormLabel"; import FormInput from "../components/FormInput"; @@ -8,16 +10,8 @@ import FieldSet from "metabase/components/FieldSet"; import PartialQueryBuilder from "../components/PartialQueryBuilder"; import { t } from "ttag"; import { formatValue } from "metabase/lib/formatting"; - -import { reduxForm } from "redux-form"; - import * as Q_DEPRECATED from "metabase/lib/query"; -import cx from "classnames"; -import Question from "metabase-lib/lib/Question"; -import Metadata from "metabase-lib/lib/metadata/Metadata"; -import Table from "metabase-lib/lib/metadata/Table"; - @reduxForm( { form: "metric", @@ -55,11 +49,7 @@ import Table from "metabase-lib/lib/metadata/Table"; ) export default class MetricForm extends Component { renderActionButtons() { - const { - invalid, - handleSubmit, - table: { db_id: databaseId, id: tableId }, - } = this.props; + const { invalid, handleSubmit } = this.props; return (
{t`Cancel`}
); } - componentDidMount() { - if (!this.props.fields.definition.value) { - this.setDefaultQuery(); - } - } - componentDidUpdate() { - if (!this.props.fields.definition.value) { - this.setDefaultQuery(); - } - } - - setDefaultQuery() { - const { - fields: { - definition: { onChange }, - }, - metadata, - table: { id: tableId, db_id: databaseId }, - updatePreviewSummary, - } = this.props; - - if (!metadata) { - // we need metadata to generate a default question - return; - } - - const query = Question.create({ databaseId, tableId, metadata }).query(); - const table = query.table(); - let queryWithFilters; - if (table.entity_type === "entity/GoogleAnalyticsTable") { - const dateField = table.fields.find(f => f.name === "ga:date"); - if (dateField) { - queryWithFilters = query - .filter(["time-interval", ["field-id", dateField.id], -365, "day"]) - .aggregate(["metric", "ga:users"]); - } - } else { - queryWithFilters = query.aggregate(["count"]); - } - - if (queryWithFilters) { - onChange(queryWithFilters.query()); - updatePreviewSummary(queryWithFilters.datasetQuery()); - } - } - render() { const { fields: { id, name, description, definition, revision_message }, - metadata, - table, handleSubmit, previewSummary, updatePreviewSummary, @@ -142,47 +84,24 @@ export default class MetricForm extends Component { title={isNewRecord ? t`Create Your Metric` : t`Edit Your Metric`} description={ isNewRecord - ? t`You can create saved metrics to add a named metric option to this table. Saved metrics include the aggregation type, the aggregated field, and optionally any filter you add. As an example, you might use this to create something like the official way of calculating "Average Price" for an Orders table.` + ? t`You can create saved metrics to add a named metric option. Saved metrics include the aggregation type, the aggregated field, and optionally any filter you add. As an example, you might use this to create something like the official way of calculating "Average Price" for an Orders table.` : t`Make changes to your metric and leave an explanatory note.` } > - {metadata && table && ( - a.short !== "rows"), - metrics: (table.metrics || []).filter( - m => m.googleAnalyics, - ), - }, - ), - }, - }) - } - tableMetadata={table} - previewSummary={ - previewSummary == null - ? "" - : t`Result: ` + formatValue(previewSummary) - } - updatePreviewSummary={updatePreviewSummary} - {...definition} - /> - )} +
- {metric.name} + + + + {metric.name} + + {description} props.value && props.value["source-table"], + wrapped: true, +}) +@withTableMetadataLoaded +@connect((state, props) => ({ metadata: getMetadata(state) })) export default class PartialQueryBuilder extends Component { static propTypes = { onChange: PropTypes.func.isRequired, - tableMetadata: PropTypes.object.isRequired, + table: PropTypes.object.isRequired, updatePreviewSummary: PropTypes.func.isRequired, previewSummary: PropTypes.string, }; componentDidMount() { - const { value, tableMetadata } = this.props; - this.props.updatePreviewSummary({ - type: "query", - database: tableMetadata.db_id, - query: { - ...value, - "source-table": tableMetadata.id, - }, - }); + const { value, table } = this.props; + if (table && value != null) { + this.props.updatePreviewSummary({ + type: "query", + database: table.db_id, + query: { + ...value, + "source-table": table.id, + }, + }); + } else { + this.maybeSetDefaultQuery(); + } + } + + componentDidUpdate() { + this.maybeSetDefaultQuery(); + } + + maybeSetDefaultQuery() { + const { metadata, table, value } = this.props; + if (value != null && !_.isEqual(Object.keys(value), ["source-table"])) { + // only set the query if it doesn't already have an aggregation or filter + return; + } + + if (!metadata || !table) { + // we need metadata and a table to generate a default question + return; + } + + const { id: tableId, db_id: databaseId } = table; + const query = Question.create({ databaseId, tableId, metadata }).query(); + // const table = query.table(); + let queryWithFilters; + if (table.entity_type === "entity/GoogleAnalyticsTable") { + const dateField = table.fields.find(f => f.name === "ga:date"); + if (dateField) { + queryWithFilters = query + .filter(["time-interval", ["field-id", dateField.id], -365, "day"]) + .aggregate(["metric", "ga:users"]); + } + } else { + queryWithFilters = query.aggregate(["count"]); + } + + if (queryWithFilters) { + this.setDatasetQuery(queryWithFilters.datasetQuery()); + } } setDatasetQuery = datasetQuery => { @@ -35,27 +86,21 @@ export default class PartialQueryBuilder extends Component { }; render() { - const { - features, - value, - metadata, - tableMetadata, - previewSummary, - } = this.props; - - const datasetQuery = { - type: "query", - database: tableMetadata.db_id, - query: { - ...value, - "source-table": tableMetadata.id, - }, - }; + const { features, value, metadata, table, previewSummary } = this.props; + + const datasetQuery = table + ? { + type: "query", + database: table.db_id, + query: value, + } + : { + type: "query", + query: {}, + }; const query = new Question( - { - dataset_query: datasetQuery, - }, + { dataset_query: datasetQuery }, metadata, ).query(); @@ -69,12 +114,10 @@ export default class PartialQueryBuilder extends Component {
{previewSummary} diff --git a/frontend/src/metabase/admin/datamodel/containers/SegmentForm.jsx b/frontend/src/metabase/admin/datamodel/components/SegmentForm.jsx similarity index 76% rename from frontend/src/metabase/admin/datamodel/containers/SegmentForm.jsx rename to frontend/src/metabase/admin/datamodel/components/SegmentForm.jsx index 77c81f1aa6e83..ce6aa4252635f 100644 --- a/frontend/src/metabase/admin/datamodel/containers/SegmentForm.jsx +++ b/frontend/src/metabase/admin/datamodel/components/SegmentForm.jsx @@ -12,8 +12,6 @@ import { formatValue } from "metabase/lib/formatting"; import { reduxForm } from "redux-form"; import cx from "classnames"; -import Metadata from "metabase-lib/lib/metadata/Metadata"; -import Table from "metabase-lib/lib/metadata/Table"; @reduxForm( { @@ -70,7 +68,7 @@ export default class SegmentForm extends Component { } renderActionButtons() { - const { invalid, handleSubmit, table } = this.props; + const { invalid, handleSubmit } = this.props; return (
{t`Cancel`}
@@ -91,8 +89,6 @@ export default class SegmentForm extends Component { render() { const { fields: { id, name, description, definition, revision_message }, - metadata, - table, handleSubmit, previewSummary, } = this.props; @@ -106,41 +102,23 @@ export default class SegmentForm extends Component { title={isNewRecord ? t`Create Your Segment` : t`Edit Your Segment`} description={ isNewRecord - ? t`Select and add filters to create your new segment for the ${table.display_name} table` + ? t`Select and add filters to create your new segment.` : t`Make changes to your segment and leave an explanatory note.` } > - {metadata && table && ( - - )} +
- {segment.name} + + + + {segment.name} + + {description} -
-

{t`Columns`}

- +
+
+ +
db.id === this.props.databaseId, - )[0]; - if (database) { - const columns = [ - { - selectedItem: database, - items: this.props.databases, - itemTitleFn: db => db.name, - itemSelectFn: db => { - this.props.selectDatabase(db); - this.refs.databasePopover.toggle(); - }, - }, - ]; - const triggerElement = ( - - {database.name} - - - ); - return ( - - - - ); - } - } - // Show a gear to access Table settings page if we're currently looking at a Table. Otherwise show nothing. // TODO - it would be nicer just to disable the gear so the page doesn't jump around once you select a Table. renderTableSettingsButton() { @@ -101,8 +69,10 @@ export default class MetadataHeader extends Component { return (
- {t`Current database:`}{" "} - {this.renderDbSelector()} + this.props.selectDatabase({ id })} + />
diff --git a/frontend/src/metabase/admin/datamodel/components/database/MetadataSchema.jsx b/frontend/src/metabase/admin/datamodel/components/database/MetadataSchema.jsx index 370c2409711a5..d6187051766c0 100644 --- a/frontend/src/metabase/admin/datamodel/components/database/MetadataSchema.jsx +++ b/frontend/src/metabase/admin/datamodel/components/database/MetadataSchema.jsx @@ -1,7 +1,7 @@ import React, { Component } from "react"; import PropTypes from "prop-types"; import { t } from "ttag"; -import withTableMetadataLoaded from "metabase/admin/datamodel/withTableMetadataLoaded"; +import withTableMetadataLoaded from "metabase/admin/datamodel/hoc/withTableMetadataLoaded"; import Tables from "metabase/entities/tables"; @Tables.load({ id: (state, { tableId }) => tableId, wrapped: true }) diff --git a/frontend/src/metabase/admin/datamodel/components/database/MetadataTable.jsx b/frontend/src/metabase/admin/datamodel/components/database/MetadataTable.jsx index a46302579de95..0b02116a60070 100644 --- a/frontend/src/metabase/admin/datamodel/components/database/MetadataTable.jsx +++ b/frontend/src/metabase/admin/datamodel/components/database/MetadataTable.jsx @@ -1,14 +1,12 @@ import React, { Component } from "react"; import PropTypes from "prop-types"; -import MetricsList from "./MetricsList"; import ColumnsList from "./ColumnsList"; -import SegmentsList from "./SegmentsList"; import { t } from "ttag"; import InputBlurChange from "metabase/components/InputBlurChange"; import Databases from "metabase/entities/databases"; import Tables from "metabase/entities/tables"; -import withTableMetadataLoaded from "metabase/admin/datamodel/withTableMetadataLoaded"; +import withTableMetadataLoaded from "metabase/admin/datamodel/hoc/withTableMetadataLoaded"; import _ from "underscore"; import cx from "classnames"; @@ -32,8 +30,6 @@ export default class MetadataTable extends Component { table: PropTypes.object, idfields: PropTypes.array, updateField: PropTypes.func.isRequired, - onRetireMetric: PropTypes.func.isRequired, - onRetireSegment: PropTypes.func.isRequired, }; componentWillMount() { @@ -116,7 +112,7 @@ export default class MetadataTable extends Component { } render() { - const { table, onRetireMetric, onRetireSegment } = this.props; + const { table } = this.props; if (!table) { return false; } @@ -145,8 +141,6 @@ export default class MetadataTable extends Component { {this.renderVisibilityWidget()}
- - {this.props.idfields && ( !m.googleAnalyics); - - return ( -
-
-

{t`Metrics`}

- - + {t`Add a Metric`} - -
- - - - - - - - - - {metrics.map(metric => ( - - ))} - -
{t`Name`}{t`Definition`}{t`Actions`}
- {metrics.length === 0 && ( -
- {t`Create metrics to add them to the View dropdown in the query builder`} -
- )} -
- ); - } -} diff --git a/frontend/src/metabase/admin/datamodel/components/database/SegmentsList.jsx b/frontend/src/metabase/admin/datamodel/components/database/SegmentsList.jsx deleted file mode 100644 index 552e280f55b3f..0000000000000 --- a/frontend/src/metabase/admin/datamodel/components/database/SegmentsList.jsx +++ /dev/null @@ -1,57 +0,0 @@ -import React, { Component } from "react"; -import PropTypes from "prop-types"; -import { Link } from "react-router"; -import { t } from "ttag"; -import SegmentItem from "./SegmentItem"; - -export default class SegmentsList extends Component { - static propTypes = { - tableMetadata: PropTypes.object.isRequired, - onRetire: PropTypes.func.isRequired, - }; - - render() { - const { onRetire, tableMetadata } = this.props; - const { segments: allSegments = [] } = tableMetadata; - const segments = allSegments.filter(s => !s.googleAnalyics); - - return ( -
-
-

{t`Segments`}

- - + {t`Add a Segment`} - -
- - - - - - - - - - {segments.map(segment => ( - - ))} - -
{t`Name`}{t`Definition`}{t`Actions`}
- {segments.length === 0 && ( -
- {t`Create segments to add them to the Filter dropdown in the query builder`} -
- )} -
- ); - } -} diff --git a/frontend/src/metabase/admin/datamodel/components/revisions/Revision.jsx b/frontend/src/metabase/admin/datamodel/components/revisions/Revision.jsx index d0698fd93442e..d495c4443bb03 100644 --- a/frontend/src/metabase/admin/datamodel/components/revisions/Revision.jsx +++ b/frontend/src/metabase/admin/datamodel/components/revisions/Revision.jsx @@ -7,9 +7,6 @@ import UserAvatar from "metabase/components/UserAvatar"; import moment from "moment"; -// TODO: "you" for current user -// TODO: show different color avatars for users that aren't me - export default class Revision extends Component { static propTypes = { objectName: PropTypes.string.isRequired, @@ -26,7 +23,7 @@ export default class Revision extends Component { if (revision.is_reversion) { return t`reverted to a previous version`; } - const changedKeys = Object.keys(revision.diff); + const changedKeys = Object.keys(revision.diff || {}); if (changedKeys.length === 1) { switch (changedKeys[0]) { case "name": @@ -54,8 +51,9 @@ export default class Revision extends Component { render() { const { revision, tableMetadata, userColor } = this.props; + let message = revision.message; - let diffKeys = Object.keys(revision.diff); + let diffKeys = Object.keys(revision.diff || {}); if (revision.is_creation) { // these are included in the diff --git a/frontend/src/metabase/admin/datamodel/components/revisions/RevisionHistory.jsx b/frontend/src/metabase/admin/datamodel/components/revisions/RevisionHistory.jsx index 59f3ea014656c..e0de828c1145f 100644 --- a/frontend/src/metabase/admin/datamodel/components/revisions/RevisionHistory.jsx +++ b/frontend/src/metabase/admin/datamodel/components/revisions/RevisionHistory.jsx @@ -18,7 +18,7 @@ export default class RevisionHistory extends Component { }; render() { - const { object, revisions, table, user } = this.props; + const { object, objectType, revisions, table, user } = this.props; let userColorAssignments = {}; if (revisions) { @@ -35,10 +35,9 @@ export default class RevisionHistory extends Component { diff --git a/frontend/src/metabase/admin/datamodel/containers/DataModelApp.jsx b/frontend/src/metabase/admin/datamodel/containers/DataModelApp.jsx new file mode 100644 index 0000000000000..bd85897d5c4ef --- /dev/null +++ b/frontend/src/metabase/admin/datamodel/containers/DataModelApp.jsx @@ -0,0 +1,48 @@ +import React from "react"; +import { push } from "react-router-redux"; +import { connect } from "react-redux"; +import { t } from "ttag"; + +import Radio from "metabase/components/Radio"; + +const mapDispatchToProps = { + onChangeTab: tab => push(`/admin/datamodel/${tab}`), +}; + +@connect( + null, + mapDispatchToProps, +) +export default class DataModelApp extends React.Component { + render() { + const { + children, + onChangeTab, + location: { pathname }, + } = this.props; + // this ugly nested ternary allows both "/segment/" and "/segments/" to work + const value = /\/segments?/.test(pathname) + ? "segments" + : /\/metrics?/.test(pathname) + ? "metrics" + : "database"; + + return ( +
+
+ +
+ {children} +
+ ); + } +} diff --git a/frontend/src/metabase/admin/datamodel/containers/MetadataEditorApp.jsx b/frontend/src/metabase/admin/datamodel/containers/MetadataEditorApp.jsx index 0ac27673cd004..3ba1a7f7a42be 100644 --- a/frontend/src/metabase/admin/datamodel/containers/MetadataEditorApp.jsx +++ b/frontend/src/metabase/admin/datamodel/containers/MetadataEditorApp.jsx @@ -13,7 +13,6 @@ import MetadataTable from "../components/database/MetadataTable"; import MetadataSchema from "../components/database/MetadataSchema"; import { metrics as Metrics, - segments as Segments, databases as Databases, fields as Fields, } from "metabase/entities"; @@ -40,8 +39,6 @@ const mapDispatchToProps = { updateField: field => Fields.actions.update(field), onRetireMetric: ({ id, ...rest }) => Metrics.actions.setArchived({ id }, true, rest), - onRetireSegment: ({ id, ...rest }) => - Segments.actions.setArchived({ id }, true, rest), }; @connect( @@ -66,7 +63,6 @@ export default class MetadataEditor extends Component { idfields: PropTypes.array, updateField: PropTypes.func.isRequired, onRetireMetric: PropTypes.func.isRequired, - onRetireSegment: PropTypes.func.isRequired, }; toggleShowSchema() { @@ -110,7 +106,6 @@ export default class MetadataEditor extends Component { idfields={this.props.idfields} updateField={this.props.updateField} onRetireMetric={this.props.onRetireMetric} - onRetireSegment={this.props.onRetireSegment} /> ) ) : ( diff --git a/frontend/src/metabase/admin/datamodel/containers/MetricApp.jsx b/frontend/src/metabase/admin/datamodel/containers/MetricApp.jsx index d4e20ea7cd658..b878d98a1ad77 100644 --- a/frontend/src/metabase/admin/datamodel/containers/MetricApp.jsx +++ b/frontend/src/metabase/admin/datamodel/containers/MetricApp.jsx @@ -3,14 +3,11 @@ import { connect } from "react-redux"; import { push } from "react-router-redux"; import MetabaseAnalytics from "metabase/lib/analytics"; -import { getMetadata } from "metabase/selectors/metadata"; import Metrics from "metabase/entities/metrics"; -import Tables from "metabase/entities/tables"; import { updatePreviewSummary } from "../datamodel"; import { getPreviewSummary } from "../selectors"; -import withTableMetadataLoaded from "../withTableMetadataLoaded"; -import MetricForm from "./MetricForm"; +import MetricForm from "../components/MetricForm"; const mapDispatchToProps = { updatePreviewSummary, @@ -20,21 +17,15 @@ const mapDispatchToProps = { }; const mapStateToProps = (state, props) => ({ - metadata: getMetadata(state), previewSummary: getPreviewSummary(state), }); @Metrics.load({ id: (state, props) => parseInt(props.params.id) }) -@Tables.load({ id: (state, props) => props.metric.table_id, wrapped: true }) -@withTableMetadataLoaded class UpdateMetricForm extends Component { onSubmit = async metric => { await this.props.updateMetric(metric); MetabaseAnalytics.trackEvent("Data Model", "Metric Updated"); - const { id: tableId, db_id: databaseId } = this.props.table; - this.props.onChangeLocation( - `/admin/datamodel/database/${databaseId}/table/${tableId}`, - ); + this.props.onChangeLocation(`/admin/datamodel/metrics`); }; render() { @@ -49,19 +40,14 @@ class UpdateMetricForm extends Component { } } -@Tables.load({ - id: (state, props) => parseInt(props.location.query.table), - wrapped: true, -}) -@withTableMetadataLoaded class CreateMetricForm extends Component { onSubmit = async metric => { - const { id: tableId, db_id: databaseId } = this.props.table; - await this.props.createMetric({ ...metric, table_id: tableId }); + await this.props.createMetric({ + ...metric, + table_id: metric.definition["source-table"], + }); MetabaseAnalytics.trackEvent("Data Model", "Metric Updated"); - this.props.onChangeLocation( - `/admin/datamodel/database/${databaseId}/table/${tableId}`, - ); + this.props.onChangeLocation(`/admin/datamodel/metrics`); }; render() { diff --git a/frontend/src/metabase/admin/datamodel/containers/MetricListApp.jsx b/frontend/src/metabase/admin/datamodel/containers/MetricListApp.jsx new file mode 100644 index 0000000000000..87690a36617b1 --- /dev/null +++ b/frontend/src/metabase/admin/datamodel/containers/MetricListApp.jsx @@ -0,0 +1,53 @@ +import React from "react"; +import { t } from "ttag"; + +import Metrics from "metabase/entities/metrics"; +import MetricItem from "metabase/admin/datamodel/components/MetricItem"; +import FilteredToUrlTable from "metabase/admin/datamodel/hoc/FilteredToUrlTable"; + +import Button from "metabase/components/Button"; +import Link from "metabase/components/Link"; + +@Metrics.loadList({ wrapped: true }) +@FilteredToUrlTable("metrics") +class MetricListApp extends React.Component { + render() { + const { metrics, tableSelector } = this.props; + + return ( +
+
+ {tableSelector} + + + +
+ + + + + + + + + + {metrics.map(metric => ( + metric.setArchived(true)} + metric={metric} + /> + ))} + +
{t`Name`}{t`Definition`}{t`Actions`}
+ {metrics.length === 0 && ( +
+ {t`Create metrics to add them to the Summarize dropdown in the query builder`} +
+ )} +
+ ); + } +} + +export default MetricListApp; diff --git a/frontend/src/metabase/admin/datamodel/containers/SegmentApp.jsx b/frontend/src/metabase/admin/datamodel/containers/SegmentApp.jsx index 7b10756ecaeb8..94e663df13189 100644 --- a/frontend/src/metabase/admin/datamodel/containers/SegmentApp.jsx +++ b/frontend/src/metabase/admin/datamodel/containers/SegmentApp.jsx @@ -3,15 +3,11 @@ import { connect } from "react-redux"; import { push } from "react-router-redux"; import MetabaseAnalytics from "metabase/lib/analytics"; - -import SegmentForm from "./SegmentForm"; +import Segments from "metabase/entities/segments"; import { updatePreviewSummary } from "../datamodel"; import { getPreviewSummary } from "../selectors"; -import withTableMetadataLoaded from "../withTableMetadataLoaded"; -import { getMetadata } from "metabase/selectors/metadata"; -import Segments from "metabase/entities/segments"; -import Tables from "metabase/entities/tables"; +import SegmentForm from "../components/SegmentForm"; const mapDispatchToProps = { updatePreviewSummary, @@ -21,21 +17,15 @@ const mapDispatchToProps = { }; const mapStateToProps = (state, props) => ({ - metadata: getMetadata(state, props), previewSummary: getPreviewSummary(state), }); @Segments.load({ id: (state, props) => parseInt(props.params.id) }) -@Tables.load({ id: (state, props) => props.segment.table_id, wrapped: true }) -@withTableMetadataLoaded class UpdateSegmentForm extends Component { onSubmit = async segment => { await this.props.updateSegment(segment); MetabaseAnalytics.trackEvent("Data Model", "Segment Updated"); - const { id: tableId, db_id: databaseId } = this.props.table; - this.props.onChangeLocation( - `/admin/datamodel/database/${databaseId}/table/${tableId}`, - ); + this.props.onChangeLocation(`/admin/datamodel/segments`); }; render() { @@ -50,19 +40,14 @@ class UpdateSegmentForm extends Component { } } -@Tables.load({ - id: (state, props) => parseInt(props.location.query.table), - wrapped: true, -}) -@withTableMetadataLoaded class CreateSegmentForm extends Component { onSubmit = async segment => { - const { id: tableId, db_id: databaseId } = this.props.table; - await this.props.createSegment({ ...segment, table_id: tableId }); + await this.props.createSegment({ + ...segment, + table_id: segment.definition["source-table"], + }); MetabaseAnalytics.trackEvent("Data Model", "Segment Updated"); - this.props.onChangeLocation( - `/admin/datamodel/database/${databaseId}/table/${tableId}`, - ); + this.props.onChangeLocation(`/admin/datamodel/segments`); }; render() { diff --git a/frontend/src/metabase/admin/datamodel/containers/SegmentListApp.jsx b/frontend/src/metabase/admin/datamodel/containers/SegmentListApp.jsx new file mode 100644 index 0000000000000..42d3eb9ad9180 --- /dev/null +++ b/frontend/src/metabase/admin/datamodel/containers/SegmentListApp.jsx @@ -0,0 +1,53 @@ +import React from "react"; +import { t } from "ttag"; + +import Segment from "metabase/entities/segments"; +import SegmentItem from "metabase/admin/datamodel/components/SegmentItem"; +import FilteredToUrlTable from "metabase/admin/datamodel/hoc/FilteredToUrlTable"; + +import Button from "metabase/components/Button"; +import Link from "metabase/components/Link"; + +@Segment.loadList({ wrapped: true }) +@FilteredToUrlTable("segments") +class SegmentListApp extends React.Component { + render() { + const { segments, tableSelector } = this.props; + + return ( +
+
+ {tableSelector} + + + +
+ + + + + + + + + + {segments.map(segment => ( + segment.setArchived(true)} + segment={segment} + /> + ))} + +
{t`Name`}{t`Definition`}{t`Actions`}
+ {segments.length === 0 && ( +
+ {t`Create segments to add them to the Filter dropdown in the query builder`} +
+ )} +
+ ); + } +} + +export default SegmentListApp; diff --git a/frontend/src/metabase/admin/datamodel/hoc/FilteredToUrlTable.jsx b/frontend/src/metabase/admin/datamodel/hoc/FilteredToUrlTable.jsx new file mode 100644 index 0000000000000..624cb85fa10dd --- /dev/null +++ b/frontend/src/metabase/admin/datamodel/hoc/FilteredToUrlTable.jsx @@ -0,0 +1,93 @@ +import React from "react"; +import { connect } from "react-redux"; +import { push } from "react-router-redux"; +import cx from "classnames"; +import { t } from "ttag"; + +import Tables from "metabase/entities/tables"; +import Icon from "metabase/components/Icon"; +import FieldSet from "metabase/components/FieldSet"; +import { DatabaseSchemaAndTableDataSelector } from "metabase/query_builder/components/DataSelector"; + +const FilteredToUrlTable = propName => ComposedComponent => + connect( + null, + { push }, + )( + class FilteredToUrlTable extends React.Component { + constructor(props) { + super(props); + const { table } = props.location.query || {}; + this.state = { tableId: table != null ? parseInt(table) : null }; + } + + setTableId = tableId => { + this.setState({ tableId }); + this.props.push({ + ...this.props.location, + query: tableId == null ? {} : { table: tableId }, + }); + }; + + render() { + const { [propName]: items, otherProps } = this.props; + const { tableId } = this.state; + const props = { + [propName]: + tableId == null + ? items + : items.filter(item => item.table_id === tableId), + tableSelector: ( + + ), + ...otherProps, + }; + return ; + } + }, + ); + +export default FilteredToUrlTable; + +@Tables.load({ + id: (state, props) => props.tableId, + loadingAndErrorWrapper: false, +}) +class TableSelector extends React.Component { + render() { + const { table, tableId, setTableId } = this.props; + return ( +
+
+ + {t`Filter by table`} + + + ) : ( + + {table && table.displayName()} + { + e.stopPropagation(); + setTableId(null); + }} + size={12} + /> + + ) + } + /> +
+
+ ); + } +} diff --git a/frontend/src/metabase/admin/datamodel/withTableMetadataLoaded.js b/frontend/src/metabase/admin/datamodel/hoc/withTableMetadataLoaded.js similarity index 79% rename from frontend/src/metabase/admin/datamodel/withTableMetadataLoaded.js rename to frontend/src/metabase/admin/datamodel/hoc/withTableMetadataLoaded.js index b81aa37213e2c..e17cbc2820e5e 100644 --- a/frontend/src/metabase/admin/datamodel/withTableMetadataLoaded.js +++ b/frontend/src/metabase/admin/datamodel/hoc/withTableMetadataLoaded.js @@ -9,9 +9,9 @@ export default ComposedComponent => { } } - componentDidUpdate({ table: { id: prevId } = {} }) { - const { table: { id: currId } = {} } = this.props; - if (currId !== prevId) { + componentDidUpdate({ table: prevTable }) { + const { table } = this.props; + if (table != null && table.id !== (prevTable || {}).id) { this.fetch(); } } diff --git a/frontend/src/metabase/admin/routes.jsx b/frontend/src/metabase/admin/routes.jsx index 9052014c3739e..b43aea8e357ef 100644 --- a/frontend/src/metabase/admin/routes.jsx +++ b/frontend/src/metabase/admin/routes.jsx @@ -22,8 +22,11 @@ import DatabaseListApp from "metabase/admin/databases/containers/DatabaseListApp import DatabaseEditApp from "metabase/admin/databases/containers/DatabaseEditApp"; // Metadata / Data model +import DataModelApp from "metabase/admin/datamodel/containers/DataModelApp"; import MetadataEditorApp from "metabase/admin/datamodel/containers/MetadataEditorApp"; +import MetricListApp from "metabase/admin/datamodel/containers/MetricListApp"; import MetricApp from "metabase/admin/datamodel/containers/MetricApp"; +import SegmentListApp from "metabase/admin/datamodel/containers/SegmentListApp"; import SegmentApp from "metabase/admin/datamodel/containers/SegmentApp"; import RevisionHistoryApp from "metabase/admin/datamodel/containers/RevisionHistoryApp"; import AdminPeopleApp from "metabase/admin/people/containers/AdminPeopleApp"; @@ -60,7 +63,7 @@ const getRoutes = (store, IsAdmin) => ( - + @@ -77,8 +80,10 @@ const getRoutes = (store, IsAdmin) => ( + + diff --git a/frontend/src/metabase/components/ColumnarSelector.css b/frontend/src/metabase/components/ColumnarSelector.css deleted file mode 100644 index e11a973321920..0000000000000 --- a/frontend/src/metabase/components/ColumnarSelector.css +++ /dev/null @@ -1,92 +0,0 @@ -.ColumnarSelector { - display: flex; - background-color: var(--color-bg-white); - font-weight: 700; -} - -.ColumnarSelector-column { - min-width: 180px; - max-height: 340px; - flex: 1; -} - -.ColumnarSelector-rows { - padding-top: 0.5rem; - padding-bottom: 0.5rem; -} - -.ColumnarSelector-title { - color: var(--color-text-medium); - text-transform: uppercase; - font-size: 10px; - font-weight: 700; - padding: var(--padding-1); - padding-left: var(--padding-3); -} - -.ColumnarSelector-section:first-child .ColumnarSelector-title { - padding-top: var(--padding-3); -} - -.ColumnarSelector-description { - margin-top: 0.5em; - color: var(--color-text-medium); - max-width: 270px; -} - -.ColumnarSelector-row { - padding-top: 0.75rem; - padding-bottom: 0.75rem; - padding-left: var(--padding-2); - padding-right: var(--padding-2); - display: flex; - align-items: center; -} - -.ColumnarSelector-row:not(.ColumnarSelector-row--disabled):hover, -.ColumnarSelector-row:not(.ColumnarSelector-row--disabled):hover .Icon { - background-color: var(--color-brand) !important; - color: white !important; -} - -.ColumnarSelector-row:not(.ColumnarSelector-row--disabled):hover - .ColumnarSelector-description { - color: color(var(--color-text-white) alpha(-50%)); -} - -.ColumnarSelector-row--selected { - color: var(--color-brand); - background: white; -} - -.ColumnarSelector-row--disabled { - color: var(--color-text-medium); -} - -.ColumnarSelector-row .Icon-check { - visibility: hidden; - padding-right: 0.5rem; -} - -.ColumnarSelector-row.ColumnarSelector-row--selected .Icon-check { - visibility: visible; -} - -.ColumnarSelector-column:first-child { - z-index: 1; -} - -/* only apply if there's more than one, aka the last is not the first */ -.ColumnarSelector-column:last-child:not(:first-child) { - background-color: white; - border-left: var(--border-size) var(--border-style) var(--color-border); - position: relative; - left: -1px; -} - -.ColumnarSelector-column:last-child .ColumnarSelector-row--selected { - background: inherit; - border-top: none; - border-bottom: none; - color: var(--color-brand); -} diff --git a/frontend/src/metabase/components/ColumnarSelector.jsx b/frontend/src/metabase/components/ColumnarSelector.jsx deleted file mode 100644 index d8b7e6665449e..0000000000000 --- a/frontend/src/metabase/components/ColumnarSelector.jsx +++ /dev/null @@ -1,99 +0,0 @@ -import React, { Component } from "react"; -import PropTypes from "prop-types"; - -import "./ColumnarSelector.css"; - -import Icon from "metabase/components/Icon"; - -import cx from "classnames"; - -export default class ColumnarSelector extends Component { - static propTypes = { - columns: PropTypes.array.isRequired, - }; - - render() { - const isItemSelected = (item, column) => - column.selectedItems - ? column.selectedItems.includes(item) - : column.selectedItem === item; - const isItemDisabled = (item, column) => - column.disabledOptionIds - ? column.disabledOptionIds.includes(item.id) - : false; - - const columns = this.props.columns.map((column, columnIndex) => { - let sectionElements; - if (column) { - const lastColumn = columnIndex === this.props.columns.length - 1; - const sections = column.sections || [column]; - sectionElements = sections.map((section, sectionIndex) => { - const title = section.title; - const items = section.items.map((item, rowIndex) => { - const itemClasses = cx({ - "ColumnarSelector-row": true, - "ColumnarSelector-row--selected": isItemSelected(item, column), - "ColumnarSelector-row--disabled": isItemDisabled(item, column), - flex: true, - "no-decoration": true, - "cursor-default": isItemDisabled(item, column), - }); - const checkIcon = lastColumn ? ( - - ) : null; - let descriptionElement; - const description = - column.itemDescriptionFn && column.itemDescriptionFn(item); - if (description) { - descriptionElement = ( -
- {description} -
- ); - } - return ( -
  • - - {checkIcon} -
    - {column.itemTitleFn(item)} - {descriptionElement} -
    -
    -
  • - ); - }); - let titleElement; - if (title) { - titleElement = ( -
    {title}
    - ); - } - return ( -
    - {titleElement} -
      {items}
    -
    - ); - }); - } - - return ( -
    - {sectionElements} -
    - ); - }); - - return
    {columns}
    ; - } -} diff --git a/frontend/src/metabase/containers/SaveQuestionModal.jsx b/frontend/src/metabase/containers/SaveQuestionModal.jsx index a72ffbe5a0800..427c1b73860e5 100644 --- a/frontend/src/metabase/containers/SaveQuestionModal.jsx +++ b/frontend/src/metabase/containers/SaveQuestionModal.jsx @@ -8,6 +8,8 @@ import ModalContent from "metabase/components/ModalContent"; import Radio from "metabase/components/Radio"; import * as Q_DEPRECATED from "metabase/lib/query"; +import { generateQueryDescription } from "metabase/lib/query/description"; + import validate from "metabase/lib/validate"; import { t } from "ttag"; @@ -76,10 +78,7 @@ export default class SaveQuestionModal extends Component { const initialValues = { name: card.name || isStructured - ? Q_DEPRECATED.generateQueryDescription( - tableMetadata, - card.dataset_query.query, - ) + ? generateQueryDescription(tableMetadata, card.dataset_query.query) : "", description: card.description || "", collection_id: diff --git a/frontend/src/metabase/css/core/text.css b/frontend/src/metabase/css/core/text.css index 28ea3af4cd94c..01f445c25e305 100644 --- a/frontend/src/metabase/css/core/text.css +++ b/frontend/src/metabase/css/core/text.css @@ -108,6 +108,10 @@ text-transform: capitalize; } +.text-transform-none { + text-transform: none; +} + /* text weight */ /* NOTE: .text-light removed since it conflicted with text-light in colors.css */ .text-normal { diff --git a/frontend/src/metabase/dashboard/components/AddSeriesModal.jsx b/frontend/src/metabase/dashboard/components/AddSeriesModal.jsx index ad00f82257890..c9a1751947f65 100644 --- a/frontend/src/metabase/dashboard/components/AddSeriesModal.jsx +++ b/frontend/src/metabase/dashboard/components/AddSeriesModal.jsx @@ -1,10 +1,10 @@ import React, { Component } from "react"; import PropTypes from "prop-types"; import { t } from "ttag"; -import _ from "underscore"; import cx from "classnames"; import { getIn } from "icepick"; import { connect } from "react-redux"; +import { createSelector } from "reselect"; import Visualization from "metabase/visualizations/components/Visualization"; import LoadingAndErrorWrapper from "metabase/components/LoadingAndErrorWrapper"; @@ -13,37 +13,29 @@ import Tooltip from "metabase/components/Tooltip"; import CheckBox from "metabase/components/CheckBox"; import MetabaseAnalytics from "metabase/lib/analytics"; -import * as Q_DEPRECATED from "metabase/lib/query"; import { color } from "metabase/lib/colors"; import Questions from "metabase/entities/questions"; +import { getMetadata } from "metabase/selectors/metadata"; +import { loadMetadataForQueries } from "metabase/redux/metadata"; + +import Question from "metabase-lib/lib/Question"; import { getVisualizationRaw } from "metabase/visualizations"; -function getQueryColumns(card, databases) { - const dbId = card.dataset_query.database; - if (card.dataset_query.type !== "query") { - return null; - } - const query = card.dataset_query.query; - const table = - databases && - databases[dbId] && - databases[dbId].tables_lookup[query["source-table"]]; - if (!table) { - return null; - } - return Q_DEPRECATED.getQueryColumns(table, query); -} +const getQuestions = createSelector( + [getMetadata, (state, ownProps) => ownProps.questions], + (metadata, questions) => + questions && questions.map(card => new Question(card, metadata)), +); // TODO: rework this so we don't have to load all cards up front +@Questions.loadList({ query: { f: "all" } }) @connect( - state => ({ - cards: Questions.selectors.getList(state, { entityQuery: { f: "all" } }), + (state, ownProps) => ({ + questions: getQuestions(state, ownProps), }), - { - fetchCards: () => Questions.actions.fetchList({ f: "all" }), - }, + { loadMetadataForQueries }, ) export default class AddSeriesModal extends Component { constructor(props, context) { @@ -53,24 +45,14 @@ export default class AddSeriesModal extends Component { searchValue: "", error: null, series: props.dashcard.series || [], - badCards: {}, + badQuestions: {}, }; - - _.bindAll( - this, - "onSearchChange", - "onSearchFocus", - "onDone", - "filteredCards", - "onRemoveSeries", - ); } static propTypes = { dashcard: PropTypes.object.isRequired, - cards: PropTypes.array, + questions: PropTypes.array, dashcardData: PropTypes.object.isRequired, - fetchCards: PropTypes.func.isRequired, fetchCardData: PropTypes.func.isRequired, fetchDatabaseMetadata: PropTypes.func.isRequired, setDashCardAttributes: PropTypes.func.isRequired, @@ -78,33 +60,30 @@ export default class AddSeriesModal extends Component { }; static defaultProps = {}; - async componentDidMount() { + async componentWillMount() { + const { questions, loadMetadataForQueries } = this.props; try { - await this.props.fetchCards(); - await Promise.all( - _.uniq(this.props.cards.map(c => c.database_id)).map(db_id => - this.props.fetchDatabaseMetadata(db_id), - ), - ); + await loadMetadataForQueries(questions.map(question => question.query())); } catch (error) { - console.error(error); + console.error("AddSeriesModal loadMetadataForQueries", error); this.setState({ error }); } } - onSearchFocus() { + handleSearchFocus = () => { MetabaseAnalytics.trackEvent("Dashboard", "Edit Series Modal", "search"); - } + }; - onSearchChange(e) { + handleSearchChange = e => { this.setState({ searchValue: e.target.value.toLowerCase() }); - } + }; - async onCardChange(card, e) { + async handleQuestionSelectedChange(question, selected) { const { dashcard, dashcardData } = this.props; const { visualization } = getVisualizationRaw([{ card: dashcard.card }]); + const card = question.card(); try { - if (e.target.checked) { + if (selected) { if (getIn(dashcardData, [dashcard.id, card.id]) === undefined) { this.setState({ state: "loading" }); await this.props.fetchCardData(card, dashcard, { @@ -139,7 +118,7 @@ export default class AddSeriesModal extends Component { } else { this.setState({ state: "incompatible", - badCards: { ...this.state.badCards, [card.id]: true }, + badQuestions: { ...this.state.badQuestions, [card.id]: true }, }); setTimeout(() => this.setState({ state: null }), 2000); @@ -157,31 +136,31 @@ export default class AddSeriesModal extends Component { MetabaseAnalytics.trackEvent("Dashboard", "Remove Series"); } } catch (e) { - console.error("onCardChange", e); + console.error("AddSeriesModal handleQuestionChange", e); this.setState({ state: "incompatible", - badCards: { ...this.state.badCards, [card.id]: true }, + badQuestions: { ...this.state.badQuestions, [card.id]: true }, }); setTimeout(() => this.setState({ state: null }), 2000); } } - onRemoveSeries(card) { + handleRemoveSeries(card) { this.setState({ series: this.state.series.filter(c => c.id !== card.id) }); MetabaseAnalytics.trackEvent("Dashboard", "Remove Series"); } - onDone() { + handleDone = () => { this.props.setDashCardAttributes({ id: this.props.dashcard.id, attributes: { series: this.state.series }, }); this.props.onClose(); MetabaseAnalytics.trackEvent("Dashboard", "Edit Series Modal", "done"); - } + }; - filteredCards() { - const { cards, dashcard, databases, dashcardData } = this.props; + filteredQuestions = () => { + const { questions, dashcard, dashcardData } = this.props; const { searchValue } = this.state; const initialSeries = { @@ -191,24 +170,30 @@ export default class AddSeriesModal extends Component { const { visualization } = getVisualizationRaw([{ card: dashcard.card }]); - return cards.filter(card => { + return questions.filter(question => { try { // filter out the card itself - if (card.id === dashcard.card.id) { + if (question.id() === dashcard.card.id) { return false; } - if (card.dataset_query.type === "query") { + if (question.isStructured()) { if ( !visualization.seriesAreCompatible(initialSeries, { - card: card, - data: { cols: getQueryColumns(card, databases), rows: [] }, + card: question.card(), + data: { cols: question.query().columns(), rows: [] }, }) ) { return false; } } // search - if (searchValue && card.name.toLowerCase().indexOf(searchValue) < 0) { + if ( + searchValue && + question + .displayName() + .toLowerCase() + .indexOf(searchValue) < 0 + ) { return false; } return true; @@ -217,24 +202,25 @@ export default class AddSeriesModal extends Component { return false; } }); - } + }; render() { - const { dashcard, dashcardData, cards } = this.props; + const { dashcard, dashcardData, questions } = this.props; + const { badQuestions } = this.state; let error = this.state.error; - let filteredCards; - if (!error && cards) { - filteredCards = this.filteredCards(); - if (filteredCards.length === 0) { + let filteredQuestions; + if (!error && questions) { + filteredQuestions = this.filteredQuestions(); + if (filteredQuestions.length === 0) { error = new Error("Whoops, no compatible questions match your search."); } // SQL cards at the bottom - filteredCards.sort((a, b) => { - if (a.dataset_query.type !== "query") { + filteredQuestions.sort((a, b) => { + if (!a.isNative()) { return 1; - } else if (b.dataset_query.type !== "query") { + } else if (!b.isNative()) { return -1; } else { return 0; @@ -242,11 +228,9 @@ export default class AddSeriesModal extends Component { }); } - const badCards = this.state.badCards; - - const enabledCards = {}; - for (const c of this.state.series) { - enabledCards[c.id] = true; + const enabledQuestions = {}; + for (const card of this.state.series) { + enabledQuestions[card.id] = true; } const series = [dashcard.card] @@ -270,7 +254,7 @@ export default class AddSeriesModal extends Component { showTitle isDashboard isMultiseries - onRemoveSeries={this.onRemoveSeries} + onRemoveSeries={this.handleRemoveSeries} /> {this.state.state && (
    -
    {() => (
      - {filteredCards.map(card => ( + {filteredQuestions.map(question => (
    • + this.handleQuestionSelectedChange( + question, + e.target.checked, + ) + } /> - {card.name} - {card.dataset_query.type !== "query" && ( + {question.displayName()} + {!question.isStructured() && ( diff --git a/frontend/src/metabase/entities/containers/EntityObjectLoader.jsx b/frontend/src/metabase/entities/containers/EntityObjectLoader.jsx index befdadbc5cb84..974b54a0fbcf1 100644 --- a/frontend/src/metabase/entities/containers/EntityObjectLoader.jsx +++ b/frontend/src/metabase/entities/containers/EntityObjectLoader.jsx @@ -98,13 +98,18 @@ export default class EntityObjectLoader extends React.Component { componentWillMount() { // $FlowFixMe: provided by @connect const { entityId, fetch } = this.props; - fetch( - { id: entityId }, - { reload: this.props.reload, properties: this.props.properties }, - ); + if (entityId != null) { + fetch( + { id: entityId }, + { reload: this.props.reload, properties: this.props.properties }, + ); + } } componentWillReceiveProps(nextProps: Props) { - if (nextProps.entityId !== this.props.entityId) { + if ( + nextProps.entityId !== this.props.entityId && + this.props.entityId != null + ) { // $FlowFixMe: provided by @connect nextProps.fetch( { id: nextProps.entityId }, @@ -133,10 +138,10 @@ export default class EntityObjectLoader extends React.Component { }; render() { // $FlowFixMe: provided by @connect - const { fetched, error, loadingAndErrorWrapper } = this.props; + const { entityId, fetched, error, loadingAndErrorWrapper } = this.props; return loadingAndErrorWrapper ? ( - FIELD_REF.isSameField(b, field, true), + FieldRef.isSameField(b, field, true), ); if (exactMatches.length > 0) { return s; } const targetMatches = query.breakout.filter(b => - FIELD_REF.isSameField(b, field, false), + FieldRef.isSameField(b, field, false), ); if (targetMatches.length > 0) { // query processor expect the order-by clause to match the breakout's datetime-field unit or fk-> target, @@ -236,287 +206,3 @@ export function getFieldOptions( return results; } - -export function formatField(fieldDef, options = {}) { - const name = stripId(fieldDef && (fieldDef.display_name || fieldDef.name)); - return name; -} - -export function getFieldName(tableMetadata, field, options) { - try { - const target = FIELD_REF.getFieldTarget(field, tableMetadata); - const components = []; - if (target.path) { - for (const fieldDef of target.path) { - components.push(formatField(fieldDef, options), " → "); - } - } - components.push(formatField(target.field, options)); - if (target.unit) { - components.push(` (${target.unit})`); - } - return components; - } catch (e) { - console.warn( - "Couldn't format field name for field", - field, - "in table", - tableMetadata, - ); - } - return "[Unknown Field]"; -} - -export function getTableDescription(tableMetadata) { - return [inflection.pluralize(tableMetadata.display_name)]; -} - -export function getAggregationDescription(tableMetadata, query, options) { - return conjunctList( - QUERY.getAggregations(query).map(aggregation => { - if (A_DEPRECATED.hasOptions(aggregation)) { - if (A_DEPRECATED.isNamed(aggregation)) { - return [A_DEPRECATED.getName(aggregation)]; - } - aggregation = A_DEPRECATED.getContent(aggregation); - } - if (A_DEPRECATED.isMetric(aggregation)) { - const metric = _.findWhere(tableMetadata.metrics, { - id: A_DEPRECATED.getMetric(aggregation), - }); - const name = metric ? metric.name : "[Unknown Metric]"; - return [ - options.jsx ? ( - {name} - ) : ( - name - ), - ]; - } - switch (aggregation[0]) { - case "rows": - return [t`Raw data`]; - case "count": - return [t`Count`]; - case "cum-count": - return [t`Cumulative count`]; - case "avg": - return [ - t`Average of `, - getFieldName(tableMetadata, aggregation[1], options), - ]; - case "distinct": - return [ - t`Distinct values of `, - getFieldName(tableMetadata, aggregation[1], options), - ]; - case "stddev": - return [ - t`Standard deviation of `, - getFieldName(tableMetadata, aggregation[1], options), - ]; - case "sum": - return [ - t`Sum of `, - getFieldName(tableMetadata, aggregation[1], options), - ]; - case "cum-sum": - return [ - t`Cumulative sum of `, - getFieldName(tableMetadata, aggregation[1], options), - ]; - case "max": - return [ - t`Maximum of `, - getFieldName(tableMetadata, aggregation[1], options), - ]; - case "min": - return [ - t`Minimum of `, - getFieldName(tableMetadata, aggregation[1], options), - ]; - default: - return [formatExpression(aggregation, { tableMetadata })]; - } - }), - "and", - ); -} - -export function getBreakoutDescription(tableMetadata, { breakout }, options) { - if (breakout && breakout.length > 0) { - return [ - t`Grouped by `, - joinList( - breakout.map(b => getFieldName(tableMetadata, b, options)), - " and ", - ), - ]; - } -} - -export function getFilterDescription(tableMetadata, query, options) { - // getFilters returns list of filters without the implied "and" - const filters = ["and"].concat(QUERY.getFilters(query)); - if (filters && filters.length > 1) { - return [ - t`Filtered by `, - getFilterClauseDescription(tableMetadata, filters, options), - ]; - } -} - -export function getFilterClauseDescription(tableMetadata, filter, options) { - if (filter[0] === "and" || filter[0] === "or") { - const clauses = filter - .slice(1) - .map(f => getFilterClauseDescription(tableMetadata, f, options)); - return conjunctList(clauses, filter[0].toLowerCase()); - } else if (filter[0] === "segment") { - const segment = _.findWhere(tableMetadata.segments, { id: filter[1] }); - const name = segment ? segment.name : "[Unknown Segment]"; - return options.jsx ? ( - {name} - ) : ( - name - ); - } else { - return getFieldName(tableMetadata, filter[1], options); - } -} - -export function getOrderByDescription(tableMetadata, query, options) { - const orderBy = query["order-by"]; - if (orderBy && orderBy.length > 0) { - return [ - t`Sorted by `, - joinList( - orderBy.map( - ([direction, field]) => - getFieldName(tableMetadata, field, options) + - " " + - (direction === "asc" ? "ascending" : "descending"), - ), - " and ", - ), - ]; - } -} - -export function getLimitDescription(tableMetadata, { limit }) { - if (limit != null) { - return [limit, " ", inflection.inflect("row", limit)]; - } -} - -export function generateQueryDescription(tableMetadata, query, options = {}) { - if (!tableMetadata) { - return ""; - } - - options = { - jsx: false, - sections: [ - "table", - "aggregation", - "breakout", - "filter", - "order-by", - "limit", - ], - ...options, - }; - - const sectionFns = { - table: getTableDescription, - aggregation: getAggregationDescription, - breakout: getBreakoutDescription, - filter: getFilterDescription, - "order-by": getOrderByDescription, - limit: getLimitDescription, - }; - - // these array gymnastics are needed to support JSX formatting - const sections = options.sections - .map(section => - _.flatten(sectionFns[section](tableMetadata, query, options)).filter( - s => !!s, - ), - ) - .filter(s => s && s.length > 0); - - const description = _.flatten(joinList(sections, ", ")); - if (options.jsx) { - return {description}; - } else { - return description.join(""); - } -} - -export function getDatetimeFieldUnit(field) { - return field && field[3]; -} - -export function getAggregationType(aggregation) { - return aggregation && aggregation[0]; -} - -export function getAggregationField(aggregation) { - return aggregation && aggregation[1]; -} - -export function getQueryColumn(tableMetadata, field) { - const target = FIELD_REF.getFieldTarget(field, tableMetadata); - const column = { ...target.field }; - if (FIELD_REF.isDatetimeField(field)) { - column.unit = getDatetimeFieldUnit(field); - } - return column; -} - -export function getQueryColumns(tableMetadata, query) { - const columns = QUERY.getBreakouts(query).map(b => - getQueryColumn(tableMetadata, b), - ); - if (QUERY.isBareRows(query)) { - if (columns.length === 0) { - return null; - } - } else { - for (const aggregation of QUERY.getAggregations(query)) { - const type = getAggregationType(aggregation); - columns.push({ - name: METRIC_NAME_BY_AGGREGATION[type], - base_type: METRIC_TYPE_BY_AGGREGATION[type], - special_type: TYPE.Number, - }); - } - } - return columns; -} - -function joinList(list, joiner) { - return _.flatten( - list.map((l, i) => (i === list.length - 1 ? [l] : [l, joiner])), - true, - ); -} - -function conjunctList(list, conjunction) { - switch (list.length) { - case 0: - return null; - case 1: - return list[0]; - case 2: - return [list[0], " ", conjunction, " ", list[1]]; - default: - return [ - list.slice(0, -1).join(", "), - ", ", - conjunction, - " ", - list[list.length - 1], - ]; - } -} diff --git a/frontend/src/metabase/lib/query/aggregation.js b/frontend/src/metabase/lib/query/aggregation.js index 3dc99e50d6cf3..061abae5e3dd9 100644 --- a/frontend/src/metabase/lib/query/aggregation.js +++ b/frontend/src/metabase/lib/query/aggregation.js @@ -1,12 +1,19 @@ /* @flow */ import { noNullValues, add, update, remove, clear } from "./util"; -import { isValidField } from "./field_ref"; +import * as FieldRef from "./field_ref"; import { STANDARD_AGGREGATIONS } from "metabase/lib/expressions"; import _ from "underscore"; -import type { AggregationClause, Aggregation } from "metabase/meta/types/Query"; +import type { + AggregationClause, + Aggregation, + AggregationWithOptions, + AggregationOptions, + ConcreteField, +} from "metabase/meta/types/Query"; +import type { MetricId } from "metabase/meta/types/Metric"; // returns canonical list of Aggregations, i.e. with deprecated "rows" removed export function getAggregations( @@ -77,18 +84,95 @@ export function hasValidAggregation(ac: ?AggregationClause): boolean { // AGGREGATION TYPES -export function isStandard(aggregation: AggregationClause): boolean { +// NOTE: these only differentiate between "standard", "metric", and "custom", but do not validate the aggregation + +export function isStandard(aggregation: any): boolean { return ( Array.isArray(aggregation) && STANDARD_AGGREGATIONS.has(aggregation[0]) && - (aggregation[1] === undefined || isValidField(aggregation[1])) + // this is needed to differentiate between "standard" aggregations with simple fields (or no field) and custom expressions, + // the latter would cause the aggregation to be considered "custom" + (aggregation[1] == null || FieldRef.isValidField(aggregation[1])) ); } -export function isMetric(aggregation: AggregationClause): boolean { +export function isMetric(aggregation: any): boolean { return Array.isArray(aggregation) && aggregation[0] === "metric"; } -export function isCustom(aggregation: AggregationClause): boolean { +export function isCustom(aggregation: any): boolean { return !isStandard(aggregation) && !isMetric(aggregation); } + +// AGGREGATION OPTIONS / NAMED AGGREGATIONS + +export function hasOptions(aggregation: any): boolean { + return Array.isArray(aggregation) && aggregation[0] === "aggregation-options"; +} +export function getOptions(aggregation: any): AggregationOptions { + return hasOptions(aggregation) ? aggregation[2] : {}; +} +export function getContent(aggregation: any): Aggregation { + return hasOptions(aggregation) ? aggregation[1] : aggregation; +} +export function isNamed(aggregation: any): boolean { + return !!getName(aggregation); +} +export function getName(aggregation: any): ?string { + return getOptions(aggregation)["display-name"]; +} +export function setName( + aggregation: any, + name: string, +): AggregationWithOptions { + return [ + "aggregation-options", + getContent(aggregation), + { "display-name": name, ...getOptions(aggregation) }, + ]; +} +export function setContent( + aggregation: any, + content: Aggregation, +): AggregationWithOptions { + return ["aggregation-options", content, getOptions(aggregation)]; +} + +// METRIC +export function getMetric(aggregation: any): ?MetricId { + if (isMetric(aggregation)) { + return aggregation[1]; + } else { + return null; + } +} + +// STANDARD + +// get the operator from a standard aggregation clause +export function getOperator(aggregation: any) { + if (isStandard(aggregation)) { + return aggregation[0]; + } else { + return null; + } +} + +// get the fieldId from a standard aggregation clause +export function getField(aggregation: any): ?ConcreteField { + if (isStandard(aggregation)) { + return aggregation[1]; + } else { + return null; + } +} + +// set the fieldId on a standard aggregation clause +export function setField(aggregation: any, fieldRef: ConcreteField) { + if (isStandard(aggregation)) { + return [aggregation[0], fieldRef]; + } else { + // TODO: is there a better failure response than just returning the aggregation unmodified?? + return aggregation; + } +} diff --git a/frontend/src/metabase/lib/query/breakout.js b/frontend/src/metabase/lib/query/breakout.js index 67b516088bb73..069c218333d06 100644 --- a/frontend/src/metabase/lib/query/breakout.js +++ b/frontend/src/metabase/lib/query/breakout.js @@ -1,10 +1,6 @@ /* @flow */ import type { Breakout, BreakoutClause } from "metabase/meta/types/Query"; -import type { TableMetadata } from "metabase/meta/types/Metadata"; -import type { Field } from "metabase/meta/types/Field"; - -import * as FIELD_REF from "./field_ref"; import { add, update, remove, clear } from "./util"; @@ -13,15 +9,6 @@ export function getBreakouts(breakouts: ?BreakoutClause): Breakout[] { return (breakouts || []).filter(b => b != null); } -export function getBreakoutFields( - breakouts: ?BreakoutClause, - tableMetadata: TableMetadata, -): Field[] { - return getBreakouts(breakouts).map( - breakout => (FIELD_REF.getFieldTarget(breakout, tableMetadata) || {}).field, - ); -} - // turns a list of Breakouts into the canonical BreakoutClause export function getBreakoutClause(breakouts: Breakout[]): ?BreakoutClause { breakouts = getBreakouts(breakouts); diff --git a/frontend/src/metabase/lib/query/description.js b/frontend/src/metabase/lib/query/description.js new file mode 100644 index 0000000000000..f308ddf008f69 --- /dev/null +++ b/frontend/src/metabase/lib/query/description.js @@ -0,0 +1,432 @@ +import React from "react"; + +import { t } from "ttag"; +import _ from "underscore"; +import inflection from "inflection"; + +import { stripId } from "metabase/lib/formatting"; + +import { format as formatExpression } from "metabase/lib/expressions/format"; + +import * as AGGREGATION from "./aggregation"; +import * as QUERY from "./query"; +import * as FIELD_REF from "./field_ref"; + +// NOTE: This doesn't support every MBQL clause, e.x. joins. It should also be moved to StructuredQuery. + +export function formatField(fieldDef, options = {}) { + const name = stripId(fieldDef && (fieldDef.display_name || fieldDef.name)); + return name; +} + +export function getFieldName(tableMetadata, field, options) { + try { + const target = FIELD_REF.getFieldTarget(field, tableMetadata); + const components = []; + if (target.path) { + for (const fieldDef of target.path) { + components.push(formatField(fieldDef, options), " → "); + } + } + components.push(formatField(target.field, options)); + if (target.unit) { + components.push(` (${target.unit})`); + } + return components; + } catch (e) { + console.warn( + "Couldn't format field name for field", + field, + "in table", + tableMetadata, + ); + } + return "[Unknown Field]"; +} + +export function getTableDescription(tableMetadata) { + return [inflection.pluralize(tableMetadata.display_name)]; +} + +export function getAggregationDescription(tableMetadata, query, options) { + return conjunctList( + QUERY.getAggregations(query).map(aggregation => { + if (AGGREGATION.hasOptions(aggregation)) { + if (AGGREGATION.isNamed(aggregation)) { + return [AGGREGATION.getName(aggregation)]; + } + aggregation = AGGREGATION.getContent(aggregation); + } + if (AGGREGATION.isMetric(aggregation)) { + const metric = _.findWhere(tableMetadata.metrics, { + id: AGGREGATION.getMetric(aggregation), + }); + const name = metric ? metric.name : "[Unknown Metric]"; + return [ + options.jsx ? ( + {name} + ) : ( + name + ), + ]; + } + switch (aggregation[0]) { + case "rows": + return [t`Raw data`]; + case "count": + return [t`Count`]; + case "cum-count": + return [t`Cumulative count`]; + case "avg": + return [ + t`Average of `, + getFieldName(tableMetadata, aggregation[1], options), + ]; + case "distinct": + return [ + t`Distinct values of `, + getFieldName(tableMetadata, aggregation[1], options), + ]; + case "stddev": + return [ + t`Standard deviation of `, + getFieldName(tableMetadata, aggregation[1], options), + ]; + case "sum": + return [ + t`Sum of `, + getFieldName(tableMetadata, aggregation[1], options), + ]; + case "cum-sum": + return [ + t`Cumulative sum of `, + getFieldName(tableMetadata, aggregation[1], options), + ]; + case "max": + return [ + t`Maximum of `, + getFieldName(tableMetadata, aggregation[1], options), + ]; + case "min": + return [ + t`Minimum of `, + getFieldName(tableMetadata, aggregation[1], options), + ]; + default: + return [formatExpression(aggregation, { tableMetadata })]; + } + }), + "and", + ); +} + +export function getBreakoutDescription(tableMetadata, { breakout }, options) { + if (breakout && breakout.length > 0) { + return [ + t`Grouped by `, + joinList( + breakout.map(b => getFieldName(tableMetadata, b, options)), + " and ", + ), + ]; + } +} + +export function getFilterDescription(tableMetadata, query, options) { + // getFilters returns list of filters without the implied "and" + const filters = ["and"].concat(QUERY.getFilters(query)); + if (filters && filters.length > 1) { + return [ + t`Filtered by `, + getFilterClauseDescription(tableMetadata, filters, options), + ]; + } +} + +export function getFilterClauseDescription(tableMetadata, filter, options) { + if (filter[0] === "and" || filter[0] === "or") { + const clauses = filter + .slice(1) + .map(f => getFilterClauseDescription(tableMetadata, f, options)); + return conjunctList(clauses, filter[0].toLowerCase()); + } else if (filter[0] === "segment") { + const segment = _.findWhere(tableMetadata.segments, { id: filter[1] }); + const name = segment ? segment.name : "[Unknown Segment]"; + return options.jsx ? ( + {name} + ) : ( + name + ); + } else { + return getFieldName(tableMetadata, filter[1], options); + } +} + +export function getOrderByDescription(tableMetadata, query, options) { + const orderBy = query["order-by"]; + if (orderBy && orderBy.length > 0) { + return [ + t`Sorted by `, + joinList( + orderBy.map( + ([direction, field]) => + getFieldName(tableMetadata, field, options) + + " " + + (direction === "asc" ? "ascending" : "descending"), + ), + " and ", + ), + ]; + } +} + +export function getLimitDescription(tableMetadata, { limit }) { + if (limit != null) { + return [limit, " ", inflection.inflect("row", limit)]; + } +} + +export function generateQueryDescription(tableMetadata, query, options = {}) { + if (!tableMetadata) { + return ""; + } + + options = { + jsx: false, + sections: [ + "table", + "aggregation", + "breakout", + "filter", + "order-by", + "limit", + ], + ...options, + }; + + const sectionFns = { + table: getTableDescription, + aggregation: getAggregationDescription, + breakout: getBreakoutDescription, + filter: getFilterDescription, + "order-by": getOrderByDescription, + limit: getLimitDescription, + }; + + // these array gymnastics are needed to support JSX formatting + const sections = options.sections + .map(section => + _.flatten(sectionFns[section](tableMetadata, query, options)).filter( + s => !!s, + ), + ) + .filter(s => s && s.length > 0); + + const description = _.flatten(joinList(sections, ", ")); + if (options.jsx) { + return {description}; + } else { + return description.join(""); + } +} + +// ----------------------------------------------------------------------------- +// These functions use the `query_description` field returned by the Segment and +// Metric APIs. They're meant for cases where you do not have the full database +// metadata available, and the server side will generate a data structure +// containing all the applicable data for formatting a user-friendly description +// of a query. +// ----------------------------------------------------------------------------- + +export function formatTableDescription({ table }, options = {}) { + return [inflection.pluralize(table)]; +} + +export function formatAggregationDescription({ aggregation }, options = {}) { + return conjunctList( + aggregation.map(agg => { + switch (agg["type"]) { + case "aggregation": + return [agg["arg"]]; + case "metric": + return [ + options.jsx ? ( + {agg["arg"]} + ) : ( + agg["arg"] + ), + ]; + case "rows": + return [t`Raw data`]; + case "count": + return [t`Count`]; + case "cum-count": + return [t`Cumulative count`]; + case "avg": + return [t`Average of `, agg["arg"]]; + case "distinct": + return [t`Distinct values of `, agg["arg"]]; + case "stddev": + return [t`Standard deviation of `, agg["arg"]]; + case "sum": + return [t`Sum of `, agg["arg"]]; + case "cum-sum": + return [t`Cumulative sum of `, agg["arg"]]; + case "max": + return [t`Maximum of `, agg["arg"]]; + case "min": + return [t`Minimum of `, agg["arg"]]; + } + }), + ); +} + +export function formatBreakoutDescription({ breakout }, options = {}) { + if (breakout && breakout.length > 0) { + return [t`Grouped by `, joinList(breakout.map(b => b), " and ")]; + } else { + return []; + } +} + +export function formatFilterDescription({ filter }, options = {}) { + if (filter && filter.length > 0) { + return [ + t`Filtered by `, + joinList( + filter.map(f => { + if (f["segment"] != null) { + return options.jsx ? ( + {f["segment"]} + ) : ( + f["segment"] + ); + } else if (f["field"] != null) { + return f["field"]; + } + }), + ", ", + ), + ]; + } else { + return []; + } +} + +export function formatOrderByDescription(parts, options = {}) { + const orderBy = parts["order-by"]; + if (orderBy && orderBy.length > 0) { + return [ + t`Sorted by `, + joinList( + orderBy.map( + field => + field["field"] + + " " + + (field["direction"] === "asc" ? "ascending" : "descending"), + ), + " and ", + ), + ]; + } else { + return []; + } +} + +export function formatLimitDescription({ limit }, options = {}) { + if (limit != null) { + return [limit, " ", inflection.inflect("row", limit)]; + } else { + return []; + } +} + +export function formatQueryDescription(parts, options = {}) { + if (!parts) { + return ""; + } + console.log("formatQueryDescription", { parts, options }); + + options = { + jsx: false, + sections: [ + "table", + "aggregation", + "breakout", + "filter", + "order-by", + "limit", + ], + ...options, + }; + + const sectionFns = { + table: formatTableDescription, + aggregation: formatAggregationDescription, + breakout: formatBreakoutDescription, + filter: formatFilterDescription, + "order-by": formatOrderByDescription, + limit: formatLimitDescription, + }; + + // these array gymnastics are needed to support JSX formatting + const sections = options.sections + .map(section => + _.flatten(sectionFns[section](parts, options)).filter(s => !!s), + ) + .filter(s => s && s.length > 0); + + const description = _.flatten(joinList(sections, ", ")); + if (options.jsx) { + return {description}; + } else { + return description.join(""); + } +} + +export function getDatetimeFieldUnit(field) { + return field && field[3]; +} + +export function getAggregationType(aggregation) { + return aggregation && aggregation[0]; +} + +export function getAggregationField(aggregation) { + return aggregation && aggregation[1]; +} + +export function getQueryColumn(tableMetadata, field) { + const target = FIELD_REF.getFieldTarget(field, tableMetadata); + const column = { ...target.field }; + if (FIELD_REF.isDatetimeField(field)) { + column.unit = getDatetimeFieldUnit(field); + } + return column; +} + +function joinList(list, joiner) { + return _.flatten( + list.map((l, i) => (i === list.length - 1 ? [l] : [l, joiner])), + true, + ); +} + +function conjunctList(list, conjunction) { + switch (list.length) { + case 0: + return null; + case 1: + return list[0]; + case 2: + return [list[0], " ", conjunction, " ", list[1]]; + default: + return [ + list.slice(0, -1).join(", "), + ", ", + conjunction, + " ", + list[list.length - 1], + ]; + } +} diff --git a/frontend/src/metabase/lib/query/query.js b/frontend/src/metabase/lib/query/query.js index f4b4adeef753e..e43380cfadf2d 100644 --- a/frontend/src/metabase/lib/query/query.js +++ b/frontend/src/metabase/lib/query/query.js @@ -19,7 +19,6 @@ import type { Field, FieldsClause, } from "metabase/meta/types/Query"; -import type { TableMetadata } from "metabase/meta/types/Metadata"; import * as A from "./aggregation"; import * as B from "./breakout"; @@ -71,9 +70,6 @@ export const removeBreakout = (query: SQ, index: number) => export const clearBreakouts = (query: SQ) => setBreakoutClause(query, B.clearBreakouts(query.breakout)); -export const getBreakoutFields = (query: SQ, tableMetadata: TableMetadata) => - B.getBreakoutFields(query.breakout, tableMetadata); - // FILTER export const getFilters = (query: SQ) => F.getFilters(query.filter); diff --git a/frontend/src/metabase/lib/query_aggregation.js b/frontend/src/metabase/lib/query_aggregation.js deleted file mode 100644 index 18c74e7bd6b04..0000000000000 --- a/frontend/src/metabase/lib/query_aggregation.js +++ /dev/null @@ -1,125 +0,0 @@ -import { isValidField } from "./query/field_ref"; - -// these are aggregations that can't only be added via custom aggregations -export const SPECIAL_AGGREGATIONS = new Set([ - "share", - "sum-where", - "count-where", - // TODO: add these to schema_metadata.js and remove from here - "var", - "median", - "percentile", -]); - -export function hasOptions(clause) { - return Array.isArray(clause) && clause[0] === "aggregation-options"; -} -export function getOptions(clause) { - return hasOptions(clause) ? clause[2] : {}; -} -export function getContent(clause) { - return hasOptions(clause) ? clause[1] : clause; -} -export function isNamed(clause) { - return getOptions(clause)["display-name"]; -} -export function getName(clause) { - return getOptions(clause)["display-name"]; -} -export function setName(clause, name) { - return [ - "aggregation-options", - getContent(clause), - { "display-name": name, ...getOptions(clause) }, - ]; -} -export function setContent(clause, content) { - return ["aggregation-options", content, getOptions(clause)]; -} - -function isValidClause(aggregation) { - return ( - Array.isArray(aggregation) && - aggregation.length > 0 && - aggregation.every(a => a != null) - ); -} - -// predicate functions -export function isStandard(aggregation) { - return ( - isValidClause(aggregation) && - !isMetric(aggregation) && - !isSpecial(aggregation) && - (aggregation.length === 1 || - (aggregation.length === 2 && isValidField(aggregation[1]))) - ); -} -export function isCustom(aggregation) { - // for now treat all named clauses as custom - return ( - isValidClause(aggregation) && - ((aggregation && hasOptions(aggregation)) || - (!isMetric(aggregation) && !isStandard(aggregation))) - ); -} - -export function isMetric(aggregation) { - return isValidClause(aggregation) && aggregation[0] === "metric"; -} - -export function isSpecial(aggregation) { - return isValidClause(aggregation) && SPECIAL_AGGREGATIONS.has(aggregation[0]); -} - -export function isValid(aggregation) { - return ( - isStandard(aggregation) || isMetric(aggregation) || isCustom(aggregation) - ); -} - -export function getAggregation(aggregation) { - return aggregation && aggregation[0]; -} - -// get the metricId from a metric aggregation clause -export function getMetric(aggregation) { - if (aggregation && isMetric(aggregation)) { - return aggregation[1]; - } else { - return null; - } -} - -// get the operator from a standard aggregation clause -export function getOperator(aggregation) { - if (aggregation && aggregation.length > 0 && aggregation[0] !== "metric") { - return aggregation[0]; - } else { - return null; - } -} - -// get the fieldId from a standard aggregation clause -export function getField(aggregation) { - if (aggregation && aggregation.length > 1 && aggregation[0] !== "metric") { - return aggregation[1]; - } else { - return null; - } -} - -// set the fieldId on a standard aggregation clause -export function setField(aggregation, fieldId) { - if ( - aggregation && - aggregation.length > 0 && - aggregation[0] && - aggregation[0] !== "metric" - ) { - return [aggregation[0], fieldId]; - } else { - // TODO: is there a better failure response than just returning the aggregation unmodified?? - return aggregation; - } -} diff --git a/frontend/src/metabase/meta/types/Query.js b/frontend/src/metabase/meta/types/Query.js index 33d38aaf4fe69..1cf27deb2c933 100644 --- a/frontend/src/metabase/meta/types/Query.js +++ b/frontend/src/metabase/meta/types/Query.js @@ -101,6 +101,16 @@ export type Aggregation = | MaxAgg | MetricAgg; +export type AggregationWithOptions = [ + "aggregation-options", + Aggregation, + AggregationOptions, +]; + +export type AggregationOptions = { + "display-name"?: string, +}; + type CountAgg = ["count"]; type CountFieldAgg = ["count", ConcreteField]; diff --git a/frontend/src/metabase/query_builder/actions.js b/frontend/src/metabase/query_builder/actions.js index 93e1418efa21e..f2f411b2de96d 100644 --- a/frontend/src/metabase/query_builder/actions.js +++ b/frontend/src/metabase/query_builder/actions.js @@ -1120,7 +1120,6 @@ export const followForeignKey = createThunkAction(FOLLOW_FOREIGN_KEY, fk => { const newCard = startNewCard("query", card.dataset_query.database); newCard.dataset_query.query["source-table"] = fk.origin.table.id; - newCard.dataset_query.query.aggregation = ["rows"]; newCard.dataset_query.query.filter = [ "and", ["=", fk.origin.id, originValue], diff --git a/frontend/src/metabase/query_builder/components/AggregationName.jsx b/frontend/src/metabase/query_builder/components/AggregationName.jsx index 730a9ab6d46a8..7a64138aa4a02 100644 --- a/frontend/src/metabase/query_builder/components/AggregationName.jsx +++ b/frontend/src/metabase/query_builder/components/AggregationName.jsx @@ -6,7 +6,7 @@ import _ from "underscore"; import { t } from "ttag"; import * as Q_DEPRECATED from "metabase/lib/query"; -import * as A_DEPRECATED from "metabase/lib/query_aggregation"; +import * as AGGREGATION from "metabase/lib/query/aggregation"; import { getAggregationOperator } from "metabase/lib/schema_metadata"; import { format } from "metabase/lib/expressions/format"; @@ -41,15 +41,15 @@ const AggregationName = ({ if (!tableMetadata) { return null; } - if (A_DEPRECATED.hasOptions(aggregation)) { - if (A_DEPRECATED.isNamed(aggregation)) { + if (AGGREGATION.hasOptions(aggregation)) { + if (AGGREGATION.isNamed(aggregation)) { return ( ); } - aggregation = A_DEPRECATED.getContent(aggregation); + aggregation = AGGREGATION.getContent(aggregation); } - return A_DEPRECATED.isCustom(aggregation) ? ( + return AGGREGATION.isCustom(aggregation) && !isRows(aggregation) ? ( - ) : A_DEPRECATED.isMetric(aggregation) ? ( + ) : AGGREGATION.isMetric(aggregation) ? ( ( - {A_DEPRECATED.getName(aggregation)} + {AGGREGATION.getName(aggregation)} ); const CustomAggregation = ({ aggregation, query, className }) => ( @@ -84,7 +84,7 @@ const CustomAggregation = ({ aggregation, query, className }) => ( ); const MetricAggregation = ({ aggregation, tableMetadata, className }) => { - const metricId = A_DEPRECATED.getMetric(aggregation); + const metricId = AGGREGATION.getMetric(aggregation); const selectedMetric = _.findWhere(tableMetadata.metrics, { id: metricId }); if (selectedMetric) { return ( @@ -103,10 +103,10 @@ const StandardAggregation = ({ customFields, className, }) => { - const fieldId = A_DEPRECATED.getField(aggregation); + const fieldId = AGGREGATION.getField(aggregation); const selectedAggregation = getAggregationOperator( - A_DEPRECATED.getOperator(aggregation), + AGGREGATION.getOperator(aggregation), ); // if this table doesn't support the selected aggregation, prompt the user to select a different one if ( @@ -137,4 +137,6 @@ const StandardAggregation = ({ } }; +const isRows = aggregation => aggregation && aggregation[0] === "rows"; + export default AggregationName; diff --git a/frontend/src/metabase/query_builder/components/AggregationPopover.jsx b/frontend/src/metabase/query_builder/components/AggregationPopover.jsx index 39d4d39181433..859c555cfda00 100644 --- a/frontend/src/metabase/query_builder/components/AggregationPopover.jsx +++ b/frontend/src/metabase/query_builder/components/AggregationPopover.jsx @@ -12,7 +12,7 @@ import QueryDefinitionTooltip from "./QueryDefinitionTooltip"; import ExpressionPopover from "./ExpressionPopover"; import * as Q_DEPRECATED from "metabase/lib/query"; -import * as A_DEPRECATED from "metabase/lib/query_aggregation"; +import * as AGGREGATION from "metabase/lib/query/aggregation"; import Aggregation from "metabase-lib/lib/queries/structured/Aggregation"; @@ -33,12 +33,12 @@ export default class AggregationPopover extends Component { choosingField: props.aggregation && props.aggregation.length > 1 && - A_DEPRECATED.isStandard(props.aggregation), + AGGREGATION.isStandard(props.aggregation), editingAggregation: props.aggregation && props.aggregation.length > 1 && - (A_DEPRECATED.isCustom(props.aggregation) || - A_DEPRECATED.isNamed(props.aggregation)), + (AGGREGATION.isCustom(props.aggregation) || + AGGREGATION.isNamed(props.aggregation)), }; } @@ -106,7 +106,7 @@ export default class AggregationPopover extends Component { if (dimension) { if (item.aggregation && item.aggregation.requiresField) { this.commitAggregation( - A_DEPRECATED.setField(item.value, dimension.mbql()), + AGGREGATION.setField(item.value, dimension.mbql()), ); } } else if (item.custom) { @@ -130,7 +130,7 @@ export default class AggregationPopover extends Component { onPickField = fieldId => { this.commitAggregation( - A_DEPRECATED.setField(this.state.aggregation, fieldId), + AGGREGATION.setField(this.state.aggregation, fieldId), ); }; @@ -166,7 +166,7 @@ export default class AggregationPopover extends Component { itemIsSelected(item) { const { aggregation } = this.props; - return item.isSelected(A_DEPRECATED.getContent(aggregation)); + return item.isSelected(AGGREGATION.getContent(aggregation)); } renderItemExtra(item, itemIndex) { @@ -224,16 +224,16 @@ export default class AggregationPopover extends Component { } const { choosingField, editingAggregation } = this.state; - const aggregation = A_DEPRECATED.getContent(this.state.aggregation); + const aggregation = AGGREGATION.getContent(this.state.aggregation); let selectedAggregation; - if (A_DEPRECATED.isMetric(aggregation)) { + if (AGGREGATION.isMetric(aggregation)) { selectedAggregation = _.findWhere(tableMetadata.metrics, { - id: A_DEPRECATED.getMetric(aggregation), + id: AGGREGATION.getMetric(aggregation), }); - } else if (A_DEPRECATED.getOperator(aggregation)) { + } else if (AGGREGATION.isStandard(aggregation)) { selectedAggregation = _.findWhere(aggregationOperators, { - short: A_DEPRECATED.getOperator(aggregation), + short: AGGREGATION.getOperator(aggregation), }); } @@ -243,26 +243,27 @@ export default class AggregationPopover extends Component { : aggregation.name, value: [aggregation.short, ...aggregation.fields.map(field => null)], isSelected: agg => - !A_DEPRECATED.isCustom(agg) && - A_DEPRECATED.getAggregation(agg) === aggregation.short, + AGGREGATION.isStandard(agg) && + AGGREGATION.getOperator(agg) === aggregation.short, aggregation: aggregation, })); // we only want to consider active metrics, with the ONE exception that if the currently selected aggregation is a // retired metric then we include it in the list to maintain continuity - const metrics = - showMetrics && tableMetadata.metrics - ? tableMetadata.metrics.filter( - metric => - !metric.archived || - (selectedAggregation && selectedAggregation.id === metric.id), - ) - : []; + const metrics = tableMetadata.metrics + ? tableMetadata.metrics.filter(metric => + showMetrics + ? !metric.archived || + (selectedAggregation && selectedAggregation.id === metric.id) + : // GA metrics are more like columns, so they should be displayed even when showMetrics is false + metric.googleAnalyics, + ) + : []; const metricItems = metrics.map(metric => ({ name: metric.name, value: ["metric", metric.id], isSelected: aggregation => - A_DEPRECATED.getMetric(aggregation) === metric.id, + AGGREGATION.getMetric(aggregation) === metric.id, metric: metric, })); @@ -309,7 +310,7 @@ export default class AggregationPopover extends Component { { name: t`Custom…`, custom: true, - isSelected: agg => A_DEPRECATED.isCustom(agg), + isSelected: agg => AGGREGATION.isCustom(agg), }, ]; } @@ -328,7 +329,7 @@ export default class AggregationPopover extends Component { startRule="aggregation" onChange={parsedExpression => this.setState({ - aggregation: A_DEPRECATED.setContent( + aggregation: AGGREGATION.setContent( this.state.aggregation, parsedExpression, ), @@ -337,11 +338,11 @@ export default class AggregationPopover extends Component { } onBack={this.onClearAggregation} onDone={() => this.commitAggregation(this.state.aggregation)} - name={A_DEPRECATED.getName(this.state.aggregation)} + name={AGGREGATION.getName(this.state.aggregation)} onChangeName={name => this.setState({ aggregation: name - ? A_DEPRECATED.setName(aggregation, name) + ? AGGREGATION.setName(aggregation, name) : aggregation, }) } diff --git a/frontend/src/metabase/query_builder/components/AggregationWidget.jsx b/frontend/src/metabase/query_builder/components/AggregationWidget.jsx index ab8804e825f4c..e1298238151db 100644 --- a/frontend/src/metabase/query_builder/components/AggregationWidget.jsx +++ b/frontend/src/metabase/query_builder/components/AggregationWidget.jsx @@ -55,6 +55,7 @@ export default class AggregationWidget extends Component { query={query} aggregation={aggregation} onChangeAggregation={this.handleChangeAggregation} + showMetrics={this.props.showMetrics} /> ); diff --git a/frontend/src/metabase/query_builder/components/FieldName.jsx b/frontend/src/metabase/query_builder/components/FieldName.jsx index 35c5c8393b383..1b12f912247c0 100644 --- a/frontend/src/metabase/query_builder/components/FieldName.jsx +++ b/frontend/src/metabase/query_builder/components/FieldName.jsx @@ -3,7 +3,7 @@ import PropTypes from "prop-types"; import { t } from "ttag"; import Clearable from "./Clearable"; -import * as Q_DEPRECATED from "metabase/lib/query"; +import * as FieldRef from "metabase/lib/query/field_ref"; import Dimension from "metabase-lib/lib/Dimension"; @@ -28,8 +28,7 @@ export default class FieldName extends Component { const matchingField = _.find( tableMetadata.fields, field => - Q_DEPRECATED.isFieldLiteral(field.id) && - field.id[1] === fieldLiteral[1], + FieldRef.isFieldLiteral(field.id) && field.id[1] === fieldLiteral[1], ); // check whether names of field literals match return (matchingField && matchingField.display_name) || fieldLiteral[1]; @@ -50,7 +49,7 @@ export default class FieldName extends Component { : Dimension.parseMBQL(field, tableMetadata && tableMetadata.metadata); if (dimension) { parts = {dimension.render()}; - } else if (Q_DEPRECATED.isFieldLiteral(field)) { + } else if (FieldRef.isFieldLiteral(field)) { // TODO Atte Keinänen 6/23/17: Move nested queries logic to Dimension subclasses // if the Field in question is a field literal, e.g. ["field-literal", , ] just use name as-is parts.push( @@ -59,8 +58,8 @@ export default class FieldName extends Component { , ); } else if ( - Q_DEPRECATED.isLocalField(field) && - Q_DEPRECATED.isFieldLiteral(field[1]) + FieldRef.isLocalField(field) && + FieldRef.isFieldLiteral(field[1]) ) { // otherwise if for some weird reason we wound up with a Field Literal inside a field ID, // e.g. ["field-id", ["field-literal", , ], still just use the name as-is @@ -79,7 +78,7 @@ export default class FieldName extends Component { const content = ( void, - setSourceTableFn?: (id: TableId) => void, - setDatasetQuery?: (datasetQuery: DatasetQuery) => void, + setDatasetQuery: (datasetQuery: DatasetQuery) => void, isShowingDataReference?: boolean, }; @@ -61,11 +55,8 @@ export default class GuiQueryEditor extends React.Component { }; static propTypes = { - databases: PropTypes.array, isShowingDataReference: PropTypes.bool.isRequired, setDatasetQuery: PropTypes.func.isRequired, - setDatabaseFn: PropTypes.func, - setSourceTableFn: PropTypes.func, features: PropTypes.object, supportMultipleAggregations: PropTypes.bool, }; @@ -175,6 +166,7 @@ export default class GuiQueryEditor extends React.Component { query.filter(filter).update(setDatasetQuery) } onClose={() => this.refs.filterPopover.close()} + showCustom={false} />
    @@ -224,6 +216,7 @@ export default class GuiQueryEditor extends React.Component { .update(setDatasetQuery) : query.removeAggregation(index).update(setDatasetQuery) } + showMetrics={false} showRawData > {this.renderAdd(null)} @@ -309,7 +302,7 @@ export default class GuiQueryEditor extends React.Component { } renderDataSection() { - const table = this.props.query.table(); + const { query, setDatasetQuery } = this.props; return (
    {t`Data`} - - {table && table.displayName()} - + {this.props.canChangeTable ? ( + + setDatasetQuery(query.setSourceTableId(tableId).datasetQuery()) + } + /> + ) : ( + + {query.table() && query.table().displayName()} + + )}
    ); } @@ -399,11 +401,6 @@ export default class GuiQueryEditor extends React.Component { } render() { - const { query } = this.props; - if (query.readOnly()) { - return
    ; - } - return (
    this.handleDimensionChange(dimension) diff --git a/frontend/src/metabase/query_builder/components/filters/pickers/DatePicker.jsx b/frontend/src/metabase/query_builder/components/filters/pickers/DatePicker.jsx index f5e8b8e14489f..417f699b00f72 100644 --- a/frontend/src/metabase/query_builder/components/filters/pickers/DatePicker.jsx +++ b/frontend/src/metabase/query_builder/components/filters/pickers/DatePicker.jsx @@ -13,7 +13,7 @@ import DateOperatorSelector from "../DateOperatorSelector"; import DateUnitSelector from "../DateUnitSelector"; import Calendar from "metabase/components/Calendar"; -import * as Q_DEPRECATED from "metabase/lib/query"; +import * as FieldRef from "metabase/lib/query/field_ref"; import type { FieldFilter, @@ -162,7 +162,7 @@ function getDateTimeField( export function getDateTimeFieldTarget( field: ConcreteField, ): LocalFieldReference | ForeignFieldReference | ExpressionReference { - if (Q_DEPRECATED.isDatetimeField(field)) { + if (FieldRef.isDatetimeField(field)) { // $FlowFixMe: return (field[1]: // $FlowFixMe: LocalFieldReference | ForeignFieldReference | ExpressionReference); diff --git a/frontend/src/metabase/reference/selectors.js b/frontend/src/metabase/reference/selectors.js index 3f9f84f1303e3..835304e5d96c3 100644 --- a/frontend/src/metabase/reference/selectors.js +++ b/frontend/src/metabase/reference/selectors.js @@ -5,7 +5,7 @@ import Dashboards from "metabase/entities/dashboards"; import * as Query from "metabase/lib/query/query"; import * as Filter from "metabase/lib/query/filter"; -import * as A_DEPRECATED from "metabase/lib/query_aggregation"; +import * as Aggregation from "metabase/lib/query/aggregation"; import { resourceListToMap } from "metabase/lib/redux"; @@ -126,7 +126,7 @@ export const getMetricQuestions = createSelector( question.dataset_query.type === "query" && _.any( Query.getAggregations(question.dataset_query.query), - aggregation => A_DEPRECATED.getMetric(aggregation) === metricId, + aggregation => Aggregation.getMetric(aggregation) === metricId, ), ) .reduce((map, question) => assoc(map, question.id, question), {}), diff --git a/frontend/test/metabase-smoketest/admin.cy.spec.js b/frontend/test/metabase-smoketest/admin.cy.spec.js index 0640e39bcbdfb..b6d3918b3618a 100644 --- a/frontend/test/metabase-smoketest/admin.cy.spec.js +++ b/frontend/test/metabase-smoketest/admin.cy.spec.js @@ -109,6 +109,7 @@ describe("metabase-smoketest > admin", () => { .last() .click(); cy.get("input[type='text']") + .wait(1) .clear() .wait(1) .type("5"); diff --git a/frontend/test/metabase-smoketest/admin_setup.cy.spec.js b/frontend/test/metabase-smoketest/admin_setup.cy.spec.js index 4917ef5490ca6..491a4b6d29aa6 100644 --- a/frontend/test/metabase-smoketest/admin_setup.cy.spec.js +++ b/frontend/test/metabase-smoketest/admin_setup.cy.spec.js @@ -343,24 +343,39 @@ describe("smoketest > admin_setup", () => { // Changing column name from Discount to Sale cy.wait(1000) - .get("input") - .eq(5) - .clear() - .wait(1) - .type("Sale"); + .get("[value='Discount amount.'") + .parent() + .parent() + .within(() => { + cy.get("input") + .first() + .wait(1) + .clear() + .wait(1) + .type("Sale"); + }); // Changing visibility of Created At column - cy.findAllByText("Everywhere") - .first() - .click(); - cy.findByText("Do not include").click({ force: true }); + cy.wait(2000) + .get("[value='The date and time an order was submitted.']") + .parent() + .parent() + .within(() => { + cy.findByText("Everywhere").click(); + }); + cy.get(".ReactVirtualized__Grid__innerScrollContainer") + .findAllByText("Do not include") + .click(); // ({ force: true }); // Changing column formatting to display USD instead of $ - cy.get(".Icon-gear") - .eq(-2) - .click(); + cy.get("[value='The total billed amount.']") + .parent() + .parent() + .within(() => { + cy.get(".Icon-gear").click(); + }); cy.findByText("Total – Field Settings"); cy.findByText("Columns").should("not.exist"); @@ -438,9 +453,12 @@ describe("smoketest > admin_setup", () => { // Configure Key cy.findByText("Metrics"); - cy.get(".Icon-gear") - .eq(6) - .click(); + cy.get("[value='Product ID'") + .parent() + .parent() + .within(() => { + cy.get(".Icon-gear").click(); + }); cy.findByText("Plain input box").click(); cy.findByText("Search box").click(); cy.findByText("Use original value").click(); @@ -775,7 +793,7 @@ describe("smoketest > admin_setup", () => { cy.get(".Icon-eye"); }); - it("should no longer be able to access tables or questions that have been restricted as user", () => { + it("should be unable to access tables or questions that have been restricted as user", () => { cy.visit("/"); // Normal user can still see everything @@ -787,6 +805,7 @@ describe("smoketest > admin_setup", () => { cy.findByText("Ask a question").click(); + cy.findByText("Simple question"); cy.findByText("Native query").should("not.exist"); signOut(); @@ -798,9 +817,9 @@ describe("smoketest > admin_setup", () => { cy.contains("A look at your Test Table table"); cy.contains("A look at your People table"); cy.contains("A look at your Reviews table").should("not.exist"); + }); - // No collection user can view Our Analytics, but not make any changes - + it("should be unable to change questions in Our analytics as no collection user", () => { cy.findByText("Browse all items").click(); cy.findByText("Everything"); @@ -812,13 +831,13 @@ describe("smoketest > admin_setup", () => { cy.findByText("Orders").click(); cy.findByText("Summarize").click(); cy.wait(1000) - .get(".Icon-int") + .findAllByText("Quantity") .eq(1) .click(); cy.findAllByText("Done").click(); - cy.wait(1000).findByText("Quantity"); cy.findByText("Product ID").should("not.exist"); + cy.wait(1000).findByText("Quantity"); cy.findAllByText("Save") .last() @@ -840,7 +859,7 @@ describe("smoketest > admin_setup", () => { // cy.findByText("Quantity").should("not.exist"); }); - it("should be able to add a sub collection as a user", () => { + it("should add a sub collection as a user", () => { cy.visit("/collection/root"); cy.wait(3000) @@ -861,7 +880,7 @@ describe("smoketest > admin_setup", () => { cy.get(".Icon-all"); }); - it("should be able to view collections I have access to, but not ones that I don't (even with URL) as user", () => { + it("should view collections I have access to, but not ones that I don't (even with URL) as user", () => { // Check access as normal user cy.visit("/collection/root"); @@ -920,7 +939,7 @@ describe("smoketest > admin_setup", () => { ); }); - it("should not be able to access question with URL, but no permissions", () => { + it("should be unable to access question with URL (if access not permitted)", () => { signOut(); signIn("nocollection"); diff --git a/frontend/test/metabase/admin/datamodel/MetricForm.unit.spec.js b/frontend/test/metabase/admin/datamodel/MetricForm.unit.spec.js deleted file mode 100644 index 54061e0e6627e..0000000000000 --- a/frontend/test/metabase/admin/datamodel/MetricForm.unit.spec.js +++ /dev/null @@ -1,45 +0,0 @@ -import React from "react"; -import "@testing-library/jest-dom/extend-expect"; -import { render, cleanup } from "@testing-library/react"; - -import { getStore } from "metabase/store"; -import normalReducers from "metabase/reducers-main"; - -import MetricForm from "metabase/admin/datamodel/containers/MetricForm"; - -import { metadata } from "__support__/sample_dataset_fixture"; - -function renderForm(props) { - const store = getStore(normalReducers); - const [table] = metadata.tablesList(); - return render(); -} - -describe("MetricForm", () => { - afterEach(cleanup); - - it("should render creation UI", () => { - const { getByText, queryByText } = renderForm(); - getByText("Create Your Metric"); - getByText(/^You can create saved metrics/); - expect(queryByText("Reason For Changes")).toBe(null); - getByText("Save changes"); - }); - - it("should render UI for existing metrics", () => { - const metric = { id: 123 }; - const { getByText } = renderForm({ metric }); - getByText("Edit Your Metric"); - getByText(/^Make changes to your metric/); - getByText("Reason For Changes"); - getByText("Save changes"); - }); - - it("should render count as the default aggregation", () => { - const updatePreviewSummary = jest.fn(); - const { getByText } = renderForm({ metadata, updatePreviewSummary }); - getByText("Count of rows"); - const [{ query }] = updatePreviewSummary.mock.calls[1]; - expect(query.aggregation).toEqual([["count"]]); - }); -}); diff --git a/frontend/test/metabase/admin/datamodel/SegmentForm.unit.spec.js b/frontend/test/metabase/admin/datamodel/SegmentForm.unit.spec.js deleted file mode 100644 index 10652d4dc0f3c..0000000000000 --- a/frontend/test/metabase/admin/datamodel/SegmentForm.unit.spec.js +++ /dev/null @@ -1,40 +0,0 @@ -import React from "react"; -import "@testing-library/jest-dom/extend-expect"; -import { render, cleanup } from "@testing-library/react"; - -import { getStore } from "metabase/store"; -import normalReducers from "metabase/reducers-main"; - -import SegmentForm from "metabase/admin/datamodel/containers/SegmentForm"; - -function renderForm(props) { - const store = getStore(normalReducers); - return render( - , - ); -} - -describe("SegmentForm", () => { - afterEach(cleanup); - - it("should render creation UI", () => { - const { getByText, queryByText } = renderForm(); - getByText("Create Your Segment"); - getByText(/^Select and add filters to create your new segment/); - expect(queryByText("Reason For Changes")).toBe(null); - getByText("Save changes"); - }); - - it("should render UI for existing segments", () => { - const segment = { id: 123 }; - const { getByText } = renderForm({ segment }); - getByText("Edit Your Segment"); - getByText(/^Make changes to your segment/); - getByText("Reason For Changes"); - getByText("Save changes"); - }); -}); diff --git a/frontend/test/metabase/lib/query.unit.spec.js b/frontend/test/metabase/lib/query.unit.spec.js index 6db9efd4ddc99..0757f528467d2 100644 --- a/frontend/test/metabase/lib/query.unit.spec.js +++ b/frontend/test/metabase/lib/query.unit.spec.js @@ -1,14 +1,8 @@ import * as Q_DEPRECATED from "metabase/lib/query"; -import * as A_DEPRECATED from "metabase/lib/query_aggregation"; import { ORDERS } from "__support__/sample_dataset_fixture"; import Utils from "metabase/lib/utils"; -const mockTableMetadata = { - display_name: "Order", - fields: [{ id: 1, display_name: "Total" }], -}; - describe("Legacy Q_DEPRECATED library", () => { describe("createQuery", () => { it("should provide a structured query with no args", () => { @@ -320,128 +314,3 @@ describe("isValidField", () => { ).toBe(true); }); }); - -describe("generateQueryDescription", () => { - it("should work with multiple aggregations", () => { - expect( - Q_DEPRECATED.generateQueryDescription(mockTableMetadata, { - "source-table": ORDERS.id, - aggregation: [["count"], ["sum", ["field-id", 1]]], - }), - ).toEqual("Orders, Count and Sum of Total"); - }); - it("should work with named aggregations", () => { - expect( - Q_DEPRECATED.generateQueryDescription(mockTableMetadata, { - "source-table": ORDERS.id, - aggregation: [ - [ - "aggregation-options", - ["sum", ["field-id", 1]], - { "display-name": "Revenue" }, - ], - ], - }), - ).toEqual("Orders, Revenue"); - }); -}); - -describe("AggregationClause", () => { - const cases = [ - [null, undefined], - [null, null], - [null, []], - [null, [null]], - [null, "ab"], - [null, ["foo", null]], - ["metric", ["metric", 123]], - ["standard", ["sum", ["field-id", 1]]], - ["standard", ["count"]], - ["custom", ["sum-where", ["field-id"]]], - ["custom", ["aggregation-options", ["count"], { "display-name": "foo" }]], - ]; - describe("isStandard", () => { - for (const [type, clause] of cases) { - const expected = type === "standard"; - it(`should return ${expected} for ${JSON.stringify(clause)}`, () => { - expect(A_DEPRECATED.isStandard(clause)).toBe(expected); - }); - } - }); - - describe("isCustom", () => { - for (const [type, clause] of cases) { - const expected = type === "custom"; - it(`should return ${expected} for ${JSON.stringify(clause)}`, () => { - expect(A_DEPRECATED.isCustom(clause)).toBe(expected); - }); - } - }); - - describe("isMetric", () => { - for (const [type, clause] of cases) { - const expected = type === "metric"; - it(`should return ${expected} for ${JSON.stringify(clause)}`, () => { - expect(A_DEPRECATED.isMetric(clause)).toBe(expected); - }); - } - }); - - describe("isValid", () => { - for (const [type, clause] of cases) { - const expected = !(type === null); - it(`should return ${expected} for ${JSON.stringify(clause)}`, () => { - expect(A_DEPRECATED.isValid(clause)).toBe(expected); - }); - } - }); - - describe("getMetric", () => { - it("should succeed on good clauses", () => { - expect(A_DEPRECATED.getMetric(["metric", 123])).toEqual(123); - }); - - it("should be null on non-metric clauses", () => { - expect(A_DEPRECATED.getMetric(["sum", 123])).toEqual(null); - }); - }); - - describe("getOperator", () => { - it("should succeed on good clauses", () => { - expect(A_DEPRECATED.getOperator(["rows"])).toEqual("rows"); // deprecated - expect(A_DEPRECATED.getOperator(["sum", 123])).toEqual("sum"); - }); - - it("should be null on metric clauses", () => { - expect(A_DEPRECATED.getOperator(["metric", 123])).toEqual(null); - }); - }); - - describe("getField", () => { - it("should succeed on good clauses", () => { - expect(A_DEPRECATED.getField(["sum", 123])).toEqual(123); - }); - - it("should be null on clauses w/out a field", () => { - expect(A_DEPRECATED.getField(["rows"])).toEqual(null); // deprecated - }); - - it("should be null on metric clauses", () => { - expect(A_DEPRECATED.getField(["metric", 123])).toEqual(null); - }); - }); - - describe("setField", () => { - it("should succeed on good clauses", () => { - expect(A_DEPRECATED.setField(["avg"], 123)).toEqual(["avg", 123]); - expect(A_DEPRECATED.setField(["sum", null], 123)).toEqual(["sum", 123]); - }); - - it("should return unmodified on metric clauses", () => { - expect(A_DEPRECATED.setField(["metric", 123], 456)).toEqual([ - "metric", - 123, - ]); - }); - }); -}); diff --git a/frontend/test/metabase/lib/query/description.unit.spec.js b/frontend/test/metabase/lib/query/description.unit.spec.js new file mode 100644 index 0000000000000..66bfa56214fca --- /dev/null +++ b/frontend/test/metabase/lib/query/description.unit.spec.js @@ -0,0 +1,33 @@ +import { generateQueryDescription } from "metabase/lib/query/description"; + +import { ORDERS } from "__support__/sample_dataset_fixture"; + +const mockTableMetadata = { + display_name: "Order", + fields: [{ id: 1, display_name: "Total" }], +}; + +describe("generateQueryDescription", () => { + it("should work with multiple aggregations", () => { + expect( + generateQueryDescription(mockTableMetadata, { + "source-table": ORDERS.id, + aggregation: [["count"], ["sum", ["field-id", 1]]], + }), + ).toEqual("Orders, Count and Sum of Total"); + }); + it("should work with named aggregations", () => { + expect( + generateQueryDescription(mockTableMetadata, { + "source-table": ORDERS.id, + aggregation: [ + [ + "aggregation-options", + ["sum", ["field-id", 1]], + { "display-name": "Revenue" }, + ], + ], + }), + ).toEqual("Orders, Revenue"); + }); +}); diff --git a/frontend/test/metabase/scenarios/admin/datamodel/metrics.cy.spec.js b/frontend/test/metabase/scenarios/admin/datamodel/metrics.cy.spec.js new file mode 100644 index 0000000000000..47fc9772a1836 --- /dev/null +++ b/frontend/test/metabase/scenarios/admin/datamodel/metrics.cy.spec.js @@ -0,0 +1,109 @@ +import { restore, signInAsAdmin, popover, modal } from "__support__/cypress"; + +describe("scenarios > admin > datamodel > metrics", () => { + before(restore); + beforeEach(() => { + signInAsAdmin(); + cy.viewport(1400, 860); + }); + + it("should create a metric", () => { + cy.visit("/admin"); + cy.contains("Data Model").click(); + cy.contains("Metrics").click(); + cy.contains("New metric").click(); + cy.contains("Select a table").click(); + popover() + .contains("Orders") + .click({ force: true }); // this shouldn't be needed, but there were issues with reordering as loads happeend + + cy.url().should("match", /metric\/create$/); + cy.contains("Create Your Metric"); + + // filter to orders with total under 100 + cy.contains("Add filters").click(); + cy.contains("Total").click(); + cy.contains("Equal to").click(); + cy.contains("Less than").click(); + cy.get('[placeholder="Enter a number"]').type("100"); + popover() + .contains("Add filter") + .click(); + + // + cy.contains("Result: 12765"); + + // fill in name/description + cy.get('[name="name"]').type("orders <100"); + cy.get('[name="description"]').type( + "Count of orders with a total under $100.", + ); + + // saving bounces you back and you see new metric in the list + cy.contains("Save changes").click(); + cy.url().should("match", /datamodel\/metrics$/); + cy.contains("orders <100"); + cy.contains("Count, Filtered by Total"); + }); + + it("should update that metric", () => { + cy.visit("/admin"); + cy.contains("Data Model").click(); + cy.contains("Metrics").click(); + + cy.contains("orders <100") + .parent() + .parent() + .find(".Icon-ellipsis") + .click(); + cy.contains("Edit Metric").click(); + + // update the filter from "< 100" to "> 10" + cy.url().should("match", /metric\/1$/); + cy.contains("Edit Your Metric"); + cy.contains(/Total\s+is less than/).click(); + popover() + .contains("Less than") + .click(); + popover() + .contains("Greater than") + .click(); + popover() + .find("input") + .type("{SelectAll}10"); + popover() + .contains("Update filter") + .click(); + + // confirm that the preview updated + cy.contains("Result: 18703"); + + // update name and description, set a revision note, and save the update + cy.get('[name="name"]') + .clear() + .type("orders >10"); + cy.get('[name="description"]') + .clear() + .type("Count of orders with a total over $10."); + cy.get('[name="revision_message"]').type("time for a change"); + cy.contains("Save changes").click(); + + // get redirected to previous page and see the new metric name + cy.url().should("match", /datamodel\/metrics$/); + cy.contains("orders >10"); + + // clean up + cy.contains("orders >10") + .parent() + .parent() + .find(".Icon-ellipsis") + .click(); + cy.contains("Retire Metric").click(); + modal() + .find("textarea") + .type("delete it"); + modal() + .contains("button", "Retire") + .click(); + }); +}); diff --git a/frontend/test/metabase/scenarios/admin/datamodel/segments.cy.spec.js b/frontend/test/metabase/scenarios/admin/datamodel/segments.cy.spec.js index ea4b5c7a79c3a..5972f589853a9 100644 --- a/frontend/test/metabase/scenarios/admin/datamodel/segments.cy.spec.js +++ b/frontend/test/metabase/scenarios/admin/datamodel/segments.cy.spec.js @@ -1,4 +1,4 @@ -import { restore, signInAsAdmin } from "__support__/cypress"; +import { restore, signInAsAdmin, popover, modal } from "__support__/cypress"; describe("scenarios > admin > datamodel > segments", () => { before(restore); @@ -10,16 +10,14 @@ describe("scenarios > admin > datamodel > segments", () => { it("should create a segment", () => { cy.visit("/admin"); cy.contains("Data Model").click(); - cy.contains("Orders").click(); + cy.contains("Segments").click(); + cy.contains("New segment").click(); + cy.contains("Select a table").click(); + popover() + .contains("Orders") + .click({ force: true }); // this shouldn't be needed, but there were issues with reordering as loads happeend - // empty state displays message - cy.contains( - "Create segments to add them to the Filter dropdown in the query builder", - ); - - // redirected to segment form - cy.contains("Add a Segment").click(); - cy.url().should("match", /segment\/create\?table=2$/); + cy.url().should("match", /segment\/create$/); cy.contains("Create Your Segment"); // filter to orders with total under 100 @@ -28,7 +26,7 @@ describe("scenarios > admin > datamodel > segments", () => { cy.contains("Equal to").click(); cy.contains("Less than").click(); cy.get('[placeholder="Enter a number"]').type("100"); - cy.get(".PopoverBody") + popover() .contains("Add filter") .click(); @@ -41,15 +39,18 @@ describe("scenarios > admin > datamodel > segments", () => { // saving bounces you back and you see new segment in the list cy.contains("Save changes").click(); - cy.url().should("match", /datamodel\/database\/1\/table\/2$/); + cy.url().should("match", /datamodel\/segments$/); cy.contains("orders <100"); cy.contains("Filtered by Total"); }); it("should update that segment", () => { - // visit table's data model page and click to edit the segment - cy.visit("/admin/datamodel/database/1/table/2"); + cy.visit("/admin"); + cy.contains("Data Model").click(); + cy.contains("Segments").click(); + cy.contains("orders <100") + .parent() .parent() .find(".Icon-ellipsis") .click(); @@ -59,14 +60,16 @@ describe("scenarios > admin > datamodel > segments", () => { cy.url().should("match", /segment\/1$/); cy.contains("Edit Your Segment"); cy.contains(/Total\s+is less than/).click(); - cy.get(".PopoverBody") + popover() .contains("Less than") .click(); - cy.get(".PopoverBody") + popover() .contains("Greater than") .click(); - cy.get(".PopoverBody input").type("{SelectAll}10"); - cy.get(".PopoverBody") + popover() + .find("input") + .type("{SelectAll}10"); + popover() .contains("Update filter") .click(); @@ -84,17 +87,20 @@ describe("scenarios > admin > datamodel > segments", () => { cy.contains("Save changes").click(); // get redirected to previous page and see the new segment name - cy.url().should("match", /datamodel\/database\/1\/table\/2$/); + cy.url().should("match", /datamodel\/segments$/); cy.contains("orders >10"); // clean up cy.contains("orders >10") + .parent() .parent() .find(".Icon-ellipsis") .click(); cy.contains("Retire Segment").click(); - cy.get(".ModalBody textarea").type("delete it"); - cy.get(".ModalBody") + modal() + .find("textarea") + .type("delete it"); + modal() .contains("button", "Retire") .click(); }); diff --git a/frontend/test/metabase/scenarios/admin/datamodel/table.cy.spec.js b/frontend/test/metabase/scenarios/admin/datamodel/table.cy.spec.js index 0a3ad1d6b70c2..177ad18a88142 100644 --- a/frontend/test/metabase/scenarios/admin/datamodel/table.cy.spec.js +++ b/frontend/test/metabase/scenarios/admin/datamodel/table.cy.spec.js @@ -147,53 +147,6 @@ describe("scenarios > admin > datamodel > table", () => { testSelect("@user_id", "People → ID", "Products → ID"); }); - it("should allow creating segments", () => { - visitAlias("@ORDERS_URL"); - - cy.contains("Add a Segment").click(); - - cy.url().should("include", "/admin/datamodel/segment/create?table=2"); - - cy.contains("Add filters to narrow your answer").click(); - cy.contains("Product ID").click(); - cy.get(`input[placeholder="Enter an ID"]`) - .type("1") - .blur(); - cy.contains("button", "Add filter").click(); - cy.contains("93 rows"); - cy.get(`input[placeholder^="Something descriptive"]`).type( - "User 1's Orders", - ); - cy.get(`textarea[placeholder^="This is a good place"]`).type( - "Segment containing only user 1's orders", - ); - cy.contains("button", "Save changes").click(); - - cy.url().should("include", "/admin/datamodel/database/1/table/2"); - cy.contains("User 1's Orders"); - }); - - it("should allow creating metrics", () => { - visitAlias("@ORDERS_URL"); - - cy.contains("Add a Metric").click(); - - cy.url().should("include", "/admin/datamodel/metric/create?table=2"); - - cy.contains("Count of rows").click(); - cy.contains("Sum of").click(); - cy.contains("Subtotal").click(); - cy.contains("Result: 1448188"); - cy.get(`input[placeholder^="Something descriptive"]`).type("Revenue"); - cy.get(`textarea[placeholder^="This is a good place"]`).type( - "Total revenue", - ); - cy.contains("button", "Save changes").click(); - - cy.url().should("include", "/admin/datamodel/database/1/table/2"); - cy.contains("Revenue"); - }); - it("should allow sorting columns", () => { visitAlias("@ORDERS_URL"); cy.contains("Column order:").click(); diff --git a/modules/drivers/sqlite/src/metabase/driver/sqlite.clj b/modules/drivers/sqlite/src/metabase/driver/sqlite.clj index e388507562f85..4461b594edca0 100644 --- a/modules/drivers/sqlite/src/metabase/driver/sqlite.clj +++ b/modules/drivers/sqlite/src/metabase/driver/sqlite.clj @@ -27,9 +27,22 @@ (driver/register! :sqlite, :parent :sql-jdbc) -(defmethod driver/supports? [:sqlite :regex] [_ _] false) -(defmethod driver/supports? [:sqlite :percentile-aggregations] [_ _] false) -(defmethod driver/supports? [:sqlite :advanced-math-expressions] [_ _] false) +;; SQLite does not support a lot of features, so do not show the options in the interface +(doseq [[feature supported?] {:right-join false + :full-join false + :regex false + :percentile-aggregations false + :advanced-math-expressions false + :standard-deviation-aggregations false}] + (defmethod driver/supports? [:sqlite feature] [_ _] supported?)) + +;; SQLite `LIKE` clauses are case-insensitive by default, and thus cannot be made case-sensitive. So let people know +;; we have this 'feature' so the frontend doesn't try to present the option to you. +(defmethod driver/supports? [:sqlite :case-sensitivity-string-filter-options] [_ _] false) + +;; HACK SQLite doesn't support ALTER TABLE ADD CONSTRAINT FOREIGN KEY and I don't have all day to work around this so +;; for now we'll just skip the foreign key stuff in the tests. +(defmethod driver/supports? [:sqlite :foreign-keys] [_ _] (not config/is-test?)) (defmethod sql-jdbc.conn/connection-details->spec :sqlite [_ {:keys [db] @@ -280,17 +293,6 @@ (sql.qp/->honeysql driver (t/local-date t)) (hsql/call :datetime (hx/literal (u.date/format-sql t))))) -;; SQLite `LIKE` clauses are case-insensitive by default, and thus cannot be made case-sensitive. So let people know -;; we have this 'feature' so the frontend doesn't try to present the option to you. -(defmethod driver/supports? [:sqlite :case-sensitivity-string-filter-options] [_ _] false) - -;; SQLite doesn't have a standard deviation function -(defmethod driver/supports? [:sqlite :standard-deviation-aggregations] [_ _] false) - -;; HACK SQLite doesn't support ALTER TABLE ADD CONSTRAINT FOREIGN KEY and I don't have all day to work around this so -;; for now we'll just skip the foreign key stuff in the tests. -(defmethod driver/supports? [:sqlite :foreign-keys] [_ _] (not config/is-test?)) - ;; SQLite defaults everything to UTC (defmethod driver.common/current-db-time-date-formatters :sqlite [_] diff --git a/modules/drivers/sqlserver/src/metabase/driver/sqlserver.clj b/modules/drivers/sqlserver/src/metabase/driver/sqlserver.clj index cc28113aa4f32..9aafebb84f6e2 100644 --- a/modules/drivers/sqlserver/src/metabase/driver/sqlserver.clj +++ b/modules/drivers/sqlserver/src/metabase/driver/sqlserver.clj @@ -25,6 +25,10 @@ (defmethod driver/supports? [:sqlserver :regex] [_ _] false) (defmethod driver/supports? [:sqlserver :percentile-aggregations] [_ _] false) +;; SQLServer LIKE clauses are case-sensitive or not based on whether the collation of the server and the columns +;; themselves. Since this isn't something we can really change in the query itself don't present the option to the +;; users in the UI +(defmethod driver/supports? [:sqlserver :case-sensitivity-string-filter-options] [_ _] false) ;; See the list here: https://docs.microsoft.com/en-us/sql/connect/jdbc/using-basic-data-types (defmethod sql-jdbc.sync/database-type->base-type :sqlserver @@ -276,11 +280,6 @@ (defmethod sql.qp/current-datetime-honeysql-form :sqlserver [_] :%getdate) -;; SQLServer LIKE clauses are case-sensitive or not based on whether the collation of the server and the columns -;; themselves. Since this isn't something we can really change in the query itself don't present the option to the -;; users in the UI -(defmethod driver/supports? [:sqlserver :case-sensitivity-string-filter-options] [_ _] false) - (defmethod sql-jdbc.sync/excluded-schemas :sqlserver [_] #{"sys" "INFORMATION_SCHEMA"}) diff --git a/resources/migrations/000_migrations.yaml b/resources/migrations/000_migrations.yaml index 664a2d2199fb9..953f72e864b93 100644 --- a/resources/migrations/000_migrations.yaml +++ b/resources/migrations/000_migrations.yaml @@ -6387,6 +6387,7 @@ databaseChangeLog: id: 169 author: camsaul comment: Added 0.36.0 + objectQuotingStrategy: ${quote_strategy} changes: - dropColumn: tableName: metabase_table diff --git a/src/metabase/api/metric.clj b/src/metabase/api/metric.clj index fa0bbfe71e921..c389b6dc14ee3 100644 --- a/src/metabase/api/metric.clj +++ b/src/metabase/api/metric.clj @@ -7,7 +7,9 @@ [events :as events] [related :as related] [util :as u]] - [metabase.api.common :as api] + [metabase.api + [common :as api] + [query-description :as qd]] [metabase.mbql.normalize :as normalize] [metabase.models [interface :as mi] @@ -45,10 +47,19 @@ (-> (api/read-check (Metric id)) (hydrate :creator))) +(defn- add-query-descriptions + [metrics] {:pre [(coll? metrics)]} + (when (some? metrics) + (for [metric metrics] + (let [table (Table (:table_id metric))] + (assoc metric + :query_description + (qd/generate-query-description table (:definition metric))))))) + (api/defendpoint GET "/:id" "Fetch `Metric` with ID." [id] - (hydrated-metric id)) + (first (add-query-descriptions [(hydrated-metric id)]))) (defn- add-db-ids "Add `:database_id` fields to `metrics` by looking them up from their `:table_id`." @@ -64,7 +75,8 @@ (as-> (db/select Metric, :archived false, {:order-by [:%lower.name]}) metrics (hydrate metrics :creator) (add-db-ids metrics) - (filter mi/can-read? metrics))) + (filter mi/can-read? metrics) + (add-query-descriptions metrics))) (defn- write-check-and-update-metric! "Check whether current user has write permissions, then update Metric with values in `body`. Publishes appropriate diff --git a/src/metabase/api/native_query_snippet.clj b/src/metabase/api/native_query_snippet.clj index 0a3d2e4a77e97..303f532efe19c 100644 --- a/src/metabase/api/native_query_snippet.clj +++ b/src/metabase/api/native_query_snippet.clj @@ -41,17 +41,19 @@ (api/defendpoint POST "/" "Create a new `NativeQuerySnippet`." - [:as {{:keys [content description name]} :body}] - {content s/Str - description (s/maybe s/Str) - name snippet/NativeQuerySnippetName} + [:as {{:keys [content description name collection_id]} :body}] + {content s/Str + description (s/maybe s/Str) + name snippet/NativeQuerySnippetName + collection_id (s/maybe su/IntGreaterThanZero)} (check-snippet-name-is-unique name) (api/check-500 (db/insert! NativeQuerySnippet - {:content content - :creator_id api/*current-user-id* - :description description - :name name}))) + {:content content + :creator_id api/*current-user-id* + :description description + :name name + :collection_id collection_id}))) (defn- write-check-and-update-snippet! "Check whether current user has write permissions, then update NativeQuerySnippet with values in `body`. Returns @@ -59,7 +61,7 @@ [id body] (let [snippet (api/write-check NativeQuerySnippet id) body-fields (u/select-keys-when body - :present #{:description} + :present #{:description :collection_id} :non-nil #{:archived :content :name}) [changes] (data/diff body-fields snippet)] (when (seq changes) @@ -70,11 +72,12 @@ (api/defendpoint PUT "/:id" "Update an existing `NativeQuerySnippet`." - [id :as {{:keys [archived content description name] :as body} :body}] - {archived (s/maybe s/Bool) - content (s/maybe s/Str) - description (s/maybe s/Str) - name (s/maybe snippet/NativeQuerySnippetName)} + [id :as {{:keys [archived content description name collection_id] :as body} :body}] + {archived (s/maybe s/Bool) + content (s/maybe s/Str) + description (s/maybe s/Str) + name (s/maybe snippet/NativeQuerySnippetName) + collection_id (s/maybe su/IntGreaterThanZero)} (write-check-and-update-snippet! id body)) diff --git a/src/metabase/api/query_description.clj b/src/metabase/api/query_description.clj new file mode 100644 index 0000000000000..bfebdc99c573e --- /dev/null +++ b/src/metabase/api/query_description.clj @@ -0,0 +1,96 @@ +(ns metabase.api.query-description + "Functions for generating human friendly query descriptions" + (:require [clojure.string :as str] + [metabase.mbql.util :as mbql.u] + [metabase.models + [field :refer [Field]] + [metric :refer [Metric]] + [segment :refer [Segment]]] + [metabase.util.i18n :as ui18n :refer [deferred-tru]])) + +(defn- get-table-description + [metadata query] + {:table (:display_name metadata)}) + +(defn- get-aggregation-description + [metadata query] + (let [field-name (fn [match] (:display_name (Field (mbql.u/field-clause->id-or-literal match))))] + (when-let [agg-matches (mbql.u/match query + [:metric arg] {:type :metric + :arg (let [metric (Metric arg)] + (if (not (str/blank? (:name metric))) + (:name metric) + (deferred-tru "[Unknown Metric]")))} + [:rows] {:type :rows} + [:count] {:type :count} + [:cum-count] {:type :cum-count} + [:avg arg] {:type :avg :arg (field-name arg)} + [:distinct arg] {:type :distinct :arg (field-name arg)} + [:stddev arg] {:type :stddev :arg (field-name arg)} + [:sum arg] {:type :sum :arg (field-name arg)} + [:cum-sum arg] {:type :cum-sum :arg (field-name arg)} + [:max arg] {:type :max :arg (field-name arg)} + [:min arg] {:type :min :arg (field-name arg)})] + {:aggregation agg-matches}))) + +(defn- get-breakout-description + [metadata query] + (when-let [breakouts (seq (:breakout query))] + {:breakout (map #(:display_name (Field %)) breakouts)})) + +(defn- get-filter-clause-description + [metadata filt] + (let [typ (first filt)] + (cond + (or (= :field-id typ) + (= :field-literal typ)) {:field (:display_name (Field (mbql.u/field-clause->id-or-literal filt)))} + + (= :segment typ) {:segment (let [segment (Segment (second filt))] + (if segment + (:name segment) + (deferred-tru "[Unknown Segment]")))} + + :else nil))) + +(defn- get-filter-description + [metadata query] + (when-let [filters (:filter query)] + {:filter (map #(get-filter-clause-description metadata %) + (mbql.u/match filters #{:field-id :field-literal :segment} &match))})) + +(defn- get-order-by-description + [metadata query] + (when-let [order-by (:order-by query)] + {:order-by (map (fn [[direction field]] + {:field (:display_name (Field (mbql.u/field-clause->id-or-literal field))) :direction direction}) + (mbql.u/match query #{:asc :desc} &match))})) + +(defn- get-limit-description + [metadata query] + (when-let [limit (:limit query)] + {:limit limit})) + +(def ^:private query-descriptor-functions + [get-table-description + get-aggregation-description + get-breakout-description + get-filter-description + get-order-by-description + get-limit-description]) + +(defn generate-query-description + "Analyze a query and return a data structure with the parts broken down for display + in the UI. + + Ex: + { + :table \"Orders\" + :filters [\"Created At\", \"Product ID\"] + :order-by [{\"Created At\" :asc}] + } + + This data structure allows the UI to format the strings appropriately (including JSX)" + [metadata query] + (apply merge + (map (fn [f] (f metadata query)) + query-descriptor-functions))) diff --git a/src/metabase/api/segment.clj b/src/metabase/api/segment.clj index 160545a18eba0..70069599b91dd 100644 --- a/src/metabase/api/segment.clj +++ b/src/metabase/api/segment.clj @@ -6,12 +6,15 @@ [events :as events] [related :as related] [util :as u]] - [metabase.api.common :as api] + [metabase.api + [common :as api] + [query-description :as qd]] [metabase.mbql.normalize :as normalize] [metabase.models [interface :as mi] [revision :as revision] - [segment :as segment :refer [Segment]]] + [segment :as segment :refer [Segment]] + [table :as table :refer [Table]]] [metabase.util [i18n :refer [trs]] [schema :as su]] @@ -43,18 +46,27 @@ (-> (api/read-check (Segment id)) (hydrate :creator))) +(defn- add-query-descriptions + [segments] {:pre [(coll? segments)]} + (when (some? segments) + (for [segment segments] + (let [table (Table (:table_id segment))] + (assoc segment + :query_description + (qd/generate-query-description table (:definition segment))))))) + (api/defendpoint GET "/:id" "Fetch `Segment` with ID." [id] - (hydrated-segment id)) + (first (log/spy :error (add-query-descriptions [(log/spy :error (hydrated-segment (log/spy :error id)))])))) (api/defendpoint GET "/" "Fetch *all* `Segments`." [] (as-> (db/select Segment, :archived false, {:order-by [[:%lower.name :asc]]}) segments (filter mi/can-read? segments) - (hydrate segments :creator))) - + (hydrate segments :creator) + (add-query-descriptions segments))) (defn- write-check-and-update-segment! "Check whether current user has write permissions, then update Segment with values in `body`. Publishes appropriate diff --git a/src/metabase/automagic_dashboards/core.clj b/src/metabase/automagic_dashboards/core.clj index 791283aec0b9c..744811b9520c7 100644 --- a/src/metabase/automagic_dashboards/core.clj +++ b/src/metabase/automagic_dashboards/core.clj @@ -996,15 +996,15 @@ ;; (no chunking). first))] (let [show (or show max-cards)] - (log/infof (trs "Applying heuristic {0} to {1}." (:rule rule) full-name)) - (log/infof (trs "Dimensions bindings:\n{0}" + (log/info (trs "Applying heuristic {0} to {1}." (:rule rule) full-name)) + (log/info (trs "Dimensions bindings:\n{0}" (->> context :dimensions (m/map-vals #(update % :matches (partial map :name))) u/pprint-to-str))) - (log/infof (trs "Using definitions:\nMetrics:\n{0}\nFilters:\n{1}" - (->> context :metrics (m/map-vals :metric) u/pprint-to-str) - (-> context :filters u/pprint-to-str))) + (log/info (trs "Using definitions:\nMetrics:\n{0}\nFilters:\n{1}" + (->> context :metrics (m/map-vals :metric) u/pprint-to-str) + (-> context :filters u/pprint-to-str))) (-> dashboard (populate/create-dashboard show) (assoc :related (related context rule) diff --git a/src/metabase/models/card.clj b/src/metabase/models/card.clj index 5024026ec1421..f8a45b220eb49 100644 --- a/src/metabase/models/card.clj +++ b/src/metabase/models/card.clj @@ -119,7 +119,7 @@ (:database query)))))) ;; make sure this Card doesn't have circular source query references (check-for-circular-source-query-references card) - (collection/check-collection-namespace card))) + (collection/check-collection-namespace Card (:collection_id card)))) (defn- post-insert [card] ;; if this Card has any native template tag parameters we need to update FieldValues for any Fields that are @@ -154,7 +154,7 @@ ;; make sure this Card doesn't have circular source query references if we're updating the query (when (:dataset_query card) (check-for-circular-source-query-references card)) - (collection/check-collection-namespace card))) + (collection/check-collection-namespace Card (:collection_id card)))) ;; Cards don't normally get deleted (they get archived instead) so this mostly affects tests (defn- pre-delete [{:keys [id]}] diff --git a/src/metabase/models/collection.clj b/src/metabase/models/collection.clj index 37a4359225c38..3718a6af1b366 100644 --- a/src/metabase/models/collection.clj +++ b/src/metabase/models/collection.clj @@ -966,14 +966,18 @@ `allowed-namespaces`), or throw an Exception. ;; Cards can only go in Collections in the default namespace (namespace = nil) - (check-collection-namespace card)" - [{collection-id :collection_id, :as object}] + (check-collection-namespace Card new-collection-id)" + [model collection-id] (when collection-id - (let [collection-namespace (keyword (db/select-one-field :namespace 'Collection :id collection-id)) - allowed-namespaces (allowed-namespaces object)] + (let [collection (or (db/select-one [Collection :namespace] :id collection-id) + (let [msg (tru "Collection does not exist.")] + (throw (ex-info msg {:status-code 404 + :errors {:collection_id msg}})))) + collection-namespace (keyword (:namespace collection)) + allowed-namespaces (allowed-namespaces model)] (when-not (contains? allowed-namespaces collection-namespace) (let [msg (tru "A {0} can only go in Collections in the {1} namespace." - (name object) + (name model) (str/join (format " %s " (tru "or")) (map #(pr-str (or % (tru "default"))) allowed-namespaces)))] (throw (ex-info msg {:status-code 400 diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj index 05e1da017018c..ffd43dcc8a205 100644 --- a/src/metabase/models/dashboard.clj +++ b/src/metabase/models/dashboard.clj @@ -58,11 +58,11 @@ (let [defaults {:parameters []} dashboard (merge defaults dashboard)] (u/prog1 dashboard - (collection/check-collection-namespace (map->DashboardInstance dashboard))))) + (collection/check-collection-namespace Dashboard (:collection_id dashboard))))) (defn- pre-update [dashboard] (u/prog1 dashboard - (collection/check-collection-namespace dashboard))) + (collection/check-collection-namespace Dashboard (:collection_id dashboard)))) (u/strict-extend (class Dashboard) models/IModel diff --git a/src/metabase/models/native_query_snippet.clj b/src/metabase/models/native_query_snippet.clj index bc4f9a1006e27..73e1d4bdf5d2e 100644 --- a/src/metabase/models/native_query_snippet.clj +++ b/src/metabase/models/native_query_snippet.clj @@ -25,7 +25,7 @@ (defn- pre-insert [snippet] (u/prog1 snippet - (collection/check-collection-namespace snippet))) + (collection/check-collection-namespace NativeQuerySnippet (:collection_id snippet)))) (defn- pre-update [{:keys [creator_id id], :as updates}] (u/prog1 updates @@ -33,7 +33,7 @@ (when (contains? updates :creator_id) (when (not= creator_id (db/select-one-field :creator_id NativeQuerySnippet :id id)) (throw (UnsupportedOperationException. (tru "You cannot update the creator_id of a NativeQuerySnippet."))))) - (collection/check-collection-namespace updates))) + (collection/check-collection-namespace NativeQuerySnippet (:collection_id updates)))) (u/strict-extend (class NativeQuerySnippet) models/IModel diff --git a/src/metabase/models/pulse.clj b/src/metabase/models/pulse.clj index a4452c9535db0..f2c199e19429e 100644 --- a/src/metabase/models/pulse.clj +++ b/src/metabase/models/pulse.clj @@ -47,11 +47,11 @@ (defn- pre-insert [notification] (u/prog1 notification - (collection/check-collection-namespace notification))) + (collection/check-collection-namespace Pulse (:collection_id notification)))) (defn- pre-update [updates] (u/prog1 updates - (collection/check-collection-namespace updates))) + (collection/check-collection-namespace Pulse (:collection_id updates)))) (defn- alert->card "Return the Card associated with an Alert, fetching it if needed, for permissions-checking purposes." diff --git a/src/metabase/query_processor/timezone.clj b/src/metabase/query_processor/timezone.clj index c92b05c89ce69..13a05bd82faad 100644 --- a/src/metabase/query_processor/timezone.clj +++ b/src/metabase/query_processor/timezone.clj @@ -61,7 +61,7 @@ (.. (t/system-clock) getZone getId)) (defn requested-timezone-id - "The timezone that we would *like* to run a query in, regardless of whether we are actaully able to do so. This is + "The timezone that we would *like* to run a query in, regardless of whether we are actually able to do so. This is always equal to the value of the `report-timezone` Setting (if it is set), otherwise the database timezone (if known), otherwise the system timezone." ^String [] diff --git a/src/metabase/sync/analyze/fingerprint/fingerprinters.clj b/src/metabase/sync/analyze/fingerprint/fingerprinters.clj index 9f0a4c97cd310..2c8627355df6c 100644 --- a/src/metabase/sync/analyze/fingerprint/fingerprinters.clj +++ b/src/metabase/sync/analyze/fingerprint/fingerprinters.clj @@ -8,6 +8,7 @@ [math :as math]] [medley.core :as m] [metabase.models.field :as field] + [metabase.query-processor.timezone :as qp.tz] [metabase.sync.analyze.classifiers.name :as classify.name] [metabase.sync.util :as sync-util] [metabase.util :as u] @@ -17,6 +18,7 @@ [redux.core :as redux]) (:import com.bigml.histogram.Histogram com.clearspring.analytics.stream.cardinality.HyperLogLogPlus + [java.time.chrono ChronoLocalDateTime ChronoZonedDateTime] java.time.temporal.Temporal)) (defn col-wise @@ -157,6 +159,8 @@ {:type {~(first field-type) fingerprint#}}))) (trs "Error generating fingerprint for {0}" (sync-util/name-for-logging field#)))))) +(declare ->temporal) + (defn- earliest ([] nil) ([acc] @@ -182,9 +186,21 @@ (extend-protocol ITemporalCoerceable nil (->temporal [_] nil) - String (->temporal [this] (u.date/parse this)) - Long (->temporal [this] (t/instant this)) - Integer (->temporal [this] (t/instant this)) + String (->temporal [this] (->temporal (u.date/parse this))) + Long (->temporal [this] (->temporal (t/instant this))) + Integer (->temporal [this] (->temporal (t/instant this))) + ;; this is challenging, because ChronoLocalDate requires a ZoneOffset + ;; to work with. Ideally, we would use the database's ZoneOffset, but + ;; we don't have access to that here. Use the JVM's systemDefault to + ;; convert this. + ChronoLocalDateTime (->temporal [this] (.toInstant this + (.. (if-let [tz (or (qp.tz/report-timezone-id-if-supported) + (qp.tz/database-timezone-id))] + (java.time.ZoneId/of tz) + (java.time.ZoneId/systemDefault)) + (getRules) + (getOffset (java.time.Instant/now))))) + ChronoZonedDateTime (->temporal [this] (.toInstant this)) Temporal (->temporal [this] this)) (deffingerprinter :type/DateTime diff --git a/src/metabase/util/i18n.clj b/src/metabase/util/i18n.clj index 08dbbe278b5a8..c0cf6779a6a07 100644 --- a/src/metabase/util/i18n.clj +++ b/src/metabase/util/i18n.clj @@ -65,7 +65,10 @@ (apply translate-user-locale format-string args)) schema.core.Schema (explain [this] - (str this))) + (str this)) + clojure.lang.Util$EquivPred + (equiv [this other] + (= (.toString this) (.toString other)))) (p.types/defrecord+ SiteLocalizedString [format-string args] Object @@ -73,7 +76,10 @@ (apply translate-site-locale format-string args)) s/Schema (explain [this] - (str this))) + (str this)) + clojure.lang.Util$EquivPred + (equiv [this other] + (= (.toString this) (.toString other)))) (defn- localized-to-json "Write a UserLocalizedString or SiteLocalizedString to the `json-generator`. This is intended for diff --git a/test/metabase/api/collection_test.clj b/test/metabase/api/collection_test.clj index b511d2c61cab9..d040b788aec28 100644 --- a/test/metabase/api/collection_test.clj +++ b/test/metabase/api/collection_test.clj @@ -566,7 +566,10 @@ Dashboard [{dashboard-id :id} {:name "My Dashboard"}]] (letfn [(only-test-items [results] (if (sequential? results) - (filter #(#{snippet-id archived-id dashboard-id} (:id %)) results) + (filter #(#{["snippet" snippet-id] + ["snippet" archived-id] + ["dashboard" dashboard-id]} ((juxt :model :id) %)) + results) results)) (only-test-item-names [results] (let [items (only-test-items results)] @@ -585,7 +588,7 @@ (testing "\nShould be able to fetch archived Snippets" (is (= ["Archived Snippet"] (only-test-item-names ((mt/user->client :rasta) :get 200 - "collection/root/items?namespace=snippets&archived=true"))))) + "collection/root/items?namespace=snippets&archived=true"))))) (testing "\nShould be able to pass ?model=snippet, even though it makes no difference in this case" (is (= ["My Snippet"] diff --git a/test/metabase/api/metric_test.clj b/test/metabase/api/metric_test.clj index d45a990b0d6c0..285e34f54747c 100644 --- a/test/metabase/api/metric_test.clj +++ b/test/metabase/api/metric_test.clj @@ -201,13 +201,14 @@ :creator_id (user->id :rasta) :creator (user-details (fetch-user :rasta)) :archived true}) - (tt/with-temp* [Database [{database-id :id}] - Table [{table-id :id} {:db_id database-id}] - Metric [{:keys [id]} {:table_id table-id}]] - ((user->client :crowberto) :delete 204 (format "metric/%d" id) :revision_message "carryon") - ;; should still be able to fetch the archived Metric - (metric-response - ((user->client :crowberto) :get 200 (format "metric/%d" id))))) + (-> (tt/with-temp* [Database [{database-id :id}] + Table [{table-id :id} {:db_id database-id}] + Metric [{:keys [id]} {:table_id table-id}]] + ((user->client :crowberto) :delete 204 (format "metric/%d" id) :revision_message "carryon") + ;; should still be able to fetch the archived Metric + (metric-response + ((user->client :crowberto) :get 200 (format "metric/%d" id)))) + (dissoc :query_description))) ;; ## GET /api/metric/:id @@ -229,11 +230,12 @@ :description "Lookin' for a blueberry" :creator_id (user->id :crowberto) :creator (user-details (fetch-user :crowberto))}) - (tt/with-temp* [Database [{database-id :id}] - Table [{table-id :id} {:db_id database-id}] - Metric [{:keys [id]} {:creator_id (user->id :crowberto) - :table_id table-id}]] - (metric-response ((user->client :rasta) :get 200 (format "metric/%d" id))))) + (-> (tt/with-temp* [Database [{database-id :id}] + Table [{table-id :id} {:db_id database-id}] + Metric [{:keys [id]} {:creator_id (user->id :crowberto) + :table_id table-id}]] + (metric-response ((user->client :rasta) :get 200 (format "metric/%d" id)))) + (dissoc :query_description))) ;; ## GET /api/metric/:id/revisions @@ -402,6 +404,10 @@ (tu/mappify (hydrate [(assoc metric-1 :database_id (data/id)) (assoc metric-2 :database_id (data/id))] :creator)) + (map #(dissoc % :query_description) ((user->client :rasta) :get 200 "metric/"))) + +(expect + [] ((user->client :rasta) :get 200 "metric/")) ;; Test related/recommended entities diff --git a/test/metabase/api/native_query_snippet_test.clj b/test/metabase/api/native_query_snippet_test.clj index c13a8e050de85..492515de43c65 100644 --- a/test/metabase/api/native_query_snippet_test.clj +++ b/test/metabase/api/native_query_snippet_test.clj @@ -3,11 +3,14 @@ (:require [clojure [string :as str] [test :refer :all]] + [metabase + [models :refer [Collection]] + [test :as mt]] [metabase.models.native-query-snippet :refer [NativeQuerySnippet]] - [metabase.test :as mt] [metabase.util.schema :as su] [schema.core :as s] - [toucan.db :as db])) + [toucan.db :as db] + [toucan.util.test :as tt])) (def ^:private test-snippet-fields [:content :creator_id :description :name]) @@ -104,12 +107,44 @@ (finally (db/delete! NativeQuerySnippet :name "test-snippet"))))) +(deftest create-snippet-in-collection-test + (testing "POST /api/native-query-snippet" + (testing "\nShould be able to create a Snippet in a Collection" + (letfn [(create! [expected-status-code collection-id] + (try + (let [response ((mt/user->client :rasta) :post expected-status-code (snippet-url) + {:name "test-snippet", :description "Just null", :content "NULL", :collection_id collection-id})] + {:response response + :db (some-> (:id response) NativeQuerySnippet)}) + (finally + (db/delete! NativeQuerySnippet :name "test-snippet"))))] + (mt/with-temp Collection [{collection-id :id} {:namespace "snippets"}] + (let [{:keys [response db]} (create! 200 collection-id)] + (testing "\nAPI response" + (is (= {:name "test-snippet", :collection_id collection-id} + (select-keys response [:name :collection_id])))) + (testing "\nobject in application DB" + (is (schema= {:collection_id (s/eq collection-id) + s/Keyword s/Any} + db))))) + + (testing "\nShould throw an error if the Collection isn't in the 'snippets' namespace" + (mt/with-temp Collection [{collection-id :id}] + (is (= {:errors {:collection_id "A NativeQuerySnippet can only go in Collections in the :snippets namespace."} + :allowed-namespaces ["snippets"] + :collection-namespace nil} + (:response (create! 400 collection-id)))))) + + (testing "\nShould throw an error if Collection does not exist" + (is (= {:errors {:collection_id "Collection does not exist."}} + (:response (create! 404 Integer/MAX_VALUE))))))))) + (deftest update-snippet-api-test (testing "PUT /api/native-query-snippet/:id" (mt/with-temp NativeQuerySnippet [snippet {:content "-- SQL comment here" :name "comment"}] (testing "update stores updated snippet" - (doseq [[message user] {"admin user should be able to update" :crowberto + (doseq [[message user] {"admin user should be able to update" :crowberto "non-admin user should be able to update" :rasta}] (testing message (let [updated-desc "Updated description." @@ -136,3 +171,35 @@ ((mt/user->client :crowberto) :put 200 (snippet-url (:id snippet)) {:creator_id (mt/user->id :rasta)}) (is (= (mt/user->id :lucky) (db/select-one-field :creator_id NativeQuerySnippet :id (:id snippet))))))))) + +(deftest update-snippet-collection-test + (testing "PUT /api/native-query-snippet/:id" + (testing "\nChange collection_id" + (tt/with-temp* [Collection [{c1-id :id, :as collection-1} {:name "a Collection", :namespace "snippets"}] + Collection [{c2-id :id, :as collection-2} {:name "another Collection", :namespace "snippets"}]] + (let [no-collection {:name "no Collection"}] + (doseq [[source dest] [[no-collection collection-1] + [collection-1 collection-2] + [collection-1 no-collection]]] + (testing (format "\nShould be able to move a Snippet from %s to %s" (:name source) (:name dest)) + (tt/with-temp NativeQuerySnippet [{snippet-id :id} {:collection_id (:id source)}] + (testing "\nresponse" + (is (= {:collection_id (:id dest)} + (-> ((mt/user->client :rasta) :put 200 (snippet-url snippet-id) {:collection_id (:id dest)}) + (select-keys [:collection_id :errors]))))) + (testing "\nvalue in app DB" + (is (= (:id dest) + (db/select-one-field :collection_id NativeQuerySnippet :id snippet-id))))))))) + + (testing "\nShould throw an error if you try to move it to a Collection not in the 'snippets' namespace" + (tt/with-temp* [Collection [{collection-id :id}] + NativeQuerySnippet [{snippet-id :id}]] + (is (= {:errors {:collection_id "A NativeQuerySnippet can only go in Collections in the :snippets namespace."} + :allowed-namespaces ["snippets"] + :collection-namespace nil} + ((mt/user->client :rasta) :put 400 (snippet-url snippet-id) {:collection_id collection-id}))))) + + (testing "\nShould throw an error if Collection does not exist" + (tt/with-temp NativeQuerySnippet [{snippet-id :id}] + (is (= {:errors {:collection_id "Collection does not exist."}} + ((mt/user->client :rasta) :put 404 (snippet-url snippet-id) {:collection_id Integer/MAX_VALUE})))))))) diff --git a/test/metabase/api/query_description_test.clj b/test/metabase/api/query_description_test.clj new file mode 100644 index 0000000000000..de5e03d0d4487 --- /dev/null +++ b/test/metabase/api/query_description_test.clj @@ -0,0 +1,81 @@ +(ns metabase.api.query-description-test + (:require [clojure.test :refer :all] + [metabase.api.query-description :as sut] + [metabase.models + [metric :refer [Metric]] + [segment :refer [Segment]] + [table :refer [Table]]] + [metabase.test :as mt] + [metabase.test.fixtures :as fixtures] + [metabase.util.i18n :as ui18n :refer [deferred-tru]] + [toucan.util.test :as tt])) + +(use-fixtures :once (fixtures/initialize :db)) + +(deftest metrics-query-test + (mt/with-db + (testing "queries" + + (testing "without any arguments, just the table" + (is (= {:table "Venues"} + (sut/generate-query-description (Table (mt/id :venues)) + (:query (mt/mbql-query :venues)))))) + + (testing "with limit" + (is (= {:table "Venues" + :limit 10} + (sut/generate-query-description (Table (mt/id :venues)) + (:query (mt/mbql-query :venues + {:limit 10})))))) + + (testing "with cumulative sum of price" + (is (= {:table "Venues" + :aggregation [{:type :cum-sum + :arg "Price"}]} + (sut/generate-query-description (Table (mt/id :venues)) + (:query (mt/mbql-query :venues + {:aggregation [[:cum-sum $price]]})))))) + (testing "with equality filter" + (is (= {:table "Venues" + :filter [{:field "Price"}]} + (sut/generate-query-description (Table (mt/id :venues)) + (:query (mt/mbql-query :venues + {:filter [:= [$price 1234]]})))))) + + (testing "with order-by clause" + (is (= {:table "Venues" + :order-by [{:field "Price" :direction :asc}]} + (sut/generate-query-description (Table (mt/id :venues)) + (:query (mt/mbql-query :venues + {:order-by [[:asc $price]]})))))) + + (testing "with an aggregation metric" + (tt/with-temp Metric [metric {:table_id (mt/id :venues) :name "Test Metric 1" + :definition {:aggregation [[:count]]}}] + (is (= {:table "Venues" + :aggregation [{:type :metric + :arg "Test Metric 1"}]} + (sut/generate-query-description (Table (mt/id :venues)) + (:query (mt/mbql-query :venues + {:aggregation [[:metric (:id metric)]]})))))) + + (is (= {:table "Venues" + :aggregation [{:type :metric + :arg (deferred-tru "[Unknown Metric]")}]} + (sut/generate-query-description (Table (mt/id :venues)) + (:query (mt/mbql-query :venues + {:aggregation [[:metric -1]]})))))) + + (testing "with segment filters" + (tt/with-temp Segment [segment {:name "Test Segment 1"}] + (is (= {:table "Venues" + :filter [{:segment "Test Segment 1"}]} + (sut/generate-query-description (Table (mt/id :venues)) + (:query (mt/mbql-query :venues + {:filter [[:segment (:id segment)]]})))))) + + (is (= {:table "Venues" + :filter [{:segment (deferred-tru "[Unknown Segment]")}]} + (sut/generate-query-description (Table (mt/id :venues)) + (:query (mt/mbql-query :venues + {:filter [[:segment -1]]}))))))))) diff --git a/test/metabase/api/segment_test.clj b/test/metabase/api/segment_test.clj index 29115ea233798..73cd28f4bee71 100644 --- a/test/metabase/api/segment_test.clj +++ b/test/metabase/api/segment_test.clj @@ -195,13 +195,14 @@ :updated_at true :archived true :definition nil} - (tt/with-temp* [Database [{database-id :id}] - Table [{table-id :id} {:db_id database-id}] - Segment [{:keys [id]} {:table_id table-id}]] - ((user->client :crowberto) :delete 204 (format "segment/%d" id) :revision_message "carryon") - ;; should still be able to fetch the archived segment - (segment-response - ((user->client :crowberto) :get 200 (format "segment/%d" id))))) + (-> (tt/with-temp* [Database [{database-id :id}] + Table [{table-id :id} {:db_id database-id}] + Segment [{:keys [id]} {:table_id table-id}]] + ((user->client :crowberto) :delete 204 (format "segment/%d" id) :revision_message "carryon") + ;; should still be able to fetch the archived segment + (segment-response + ((user->client :crowberto) :get 200 (format "segment/%d" id)))) + (dissoc :query_description))) ;; ## GET /api/segment/:id @@ -227,12 +228,13 @@ :updated_at true :archived false :definition {:filter ["=" ["field-id" 2] "cans"]}} - (tt/with-temp* [Database [{database-id :id}] - Table [{table-id :id} {:db_id database-id}] - Segment [{:keys [id]} {:creator_id (user->id :crowberto) - :table_id table-id - :definition {:filter [:= [:field-id 2] "cans"]}}]] - (segment-response ((user->client :rasta) :get 200 (format "segment/%d" id))))) + (-> (tt/with-temp* [Database [{database-id :id}] + Table [{table-id :id} {:db_id database-id}] + Segment [{:keys [id]} {:creator_id (user->id :crowberto) + :table_id table-id + :definition {:filter [:= [:field-id 2] "cans"]}}]] + (segment-response ((user->client :rasta) :get 200 (format "segment/%d" id)))) + (dissoc :query_description))) ;; ## GET /api/segment/:id/revisions @@ -381,6 +383,10 @@ Segment [_ {:archived true}]] ; inactive segments shouldn't show up (tu/mappify (hydrate [segment-1 segment-2] :creator)) + (map #(dissoc % :query_description) ((user->client :rasta) :get 200 "segment/"))) + +(expect + [] ((user->client :rasta) :get 200 "segment/")) diff --git a/test/metabase/api/table_test.clj b/test/metabase/api/table_test.clj index 2e41fe62584aa..867dcda1edce5 100644 --- a/test/metabase/api/table_test.clj +++ b/test/metabase/api/table_test.clj @@ -269,7 +269,7 @@ :description "What a nice table!"}) (is (= (merge (-> (table-defaults) - (dissoc :segments :field_values :metrics) + (dissoc :segments :field_values :metrics :updated_at) (assoc-in [:db :details] (:details (mt/db)))) (db/select-one [Table :id :schema :name :created_at] :id (u/get-id table)) {:description "What a nice table!" diff --git a/test/metabase/automagic_dashboards/core_test.clj b/test/metabase/automagic_dashboards/core_test.clj index bc9198f3efce9..af0f2780be606 100644 --- a/test/metabase/automagic_dashboards/core_test.clj +++ b/test/metabase/automagic_dashboards/core_test.clj @@ -90,6 +90,14 @@ count (= 1))))) +(deftest wierd-characters-in-names-test + (mt/with-log-level :info + (mt/with-test-user :rasta + (with-dashboard-cleanup + (-> (Table (mt/id :venues)) + (assoc :display_name "%Venues") + test-automagic-analysis))))) + (expect (mt/with-test-user :rasta (with-dashboard-cleanup diff --git a/test/metabase/models/collection_test.clj b/test/metabase/models/collection_test.clj index 6e356b7f19267..00b0f8bfc2dee 100644 --- a/test/metabase/models/collection_test.clj +++ b/test/metabase/models/collection_test.clj @@ -1477,3 +1477,26 @@ :name "Personal Collection" :namespace "x" :personal_owner_id user-id})))))) + +(deftest check-collection-namespace-test + (testing "check-collection-namespace" + (testing "Should succeed if namespace is allowed" + (mt/with-temp* [Card [{card-id :id}] + Collection [{collection-id :id}]] + (is (= nil + (collection/check-collection-namespace Card collection-id))))) + + (testing "Should throw exception if namespace is not allowed" + (mt/with-temp* [Card [{card-id :id}] + Collection [{collection-id :id} {:namespace "x"}]] + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"A Card can only go in Collections in the \"default\" namespace" + (collection/check-collection-namespace Card collection-id))))) + + (testing "Should throw exception if Collection does not exist" + (mt/with-temp Card [{card-id :id}] + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Collection does not exist" + (collection/check-collection-namespace Card Integer/MAX_VALUE))))))) diff --git a/test/metabase/sync/analyze/fingerprint/fingerprinters_test.clj b/test/metabase/sync/analyze/fingerprint/fingerprinters_test.clj index 8b7d8cebb319f..a7d034e6002fe 100644 --- a/test/metabase/sync/analyze/fingerprint/fingerprinters_test.clj +++ b/test/metabase/sync/analyze/fingerprint/fingerprinters_test.clj @@ -1,24 +1,58 @@ (ns metabase.sync.analyze.fingerprint.fingerprinters-test (:require [clojure.test :refer :all] [metabase.models.field :as field] - [metabase.sync.analyze.fingerprint.fingerprinters :refer :all])) + [metabase.sync.analyze.fingerprint.fingerprinters :refer :all] + [metabase.test :as mt])) (deftest fingerprint-temporal-values-test - (is (= {:global {:distinct-count 4 - :nil% 0.5} - :type {:type/DateTime {:earliest "2013-01-01" - :latest "2018-01-01"}}} - (transduce identity - (fingerprinter (field/map->FieldInstance {:base_type :type/DateTime})) - [#t "2013" nil #t "2018" nil nil #t "2015"]))) - (testing "nil temporal values" - {:global {:distinct-count 1 - :nil% 1.0} - :type {:type/DateTime {:earliest nil - :latest nil}}} - (transduce identity - (fingerprinter (field/map->FieldInstance {:base_type :type/DateTime})) - (repeat 10 nil)))) + ;; we want to test h2 and postgres, because h2 doesn't + ;; support overriding the timezone for a session / report + (mt/test-drivers #{:h2 :postgres} + (doseq [tz ["UTC" nil]] + (mt/with-temporary-setting-values [report-timezone tz] + (mt/with-database-timezone-id "UTC" + (mt/with-everything-store + (is (= {:global {:distinct-count 4 + :nil% 0.5} + :type {:type/DateTime {:earliest "2013-01-01" + :latest "2018-01-01"}}} + (transduce identity + (fingerprinter (field/map->FieldInstance {:base_type :type/DateTime})) + [#t "2013" nil #t "2018" nil nil #t "2015"]))) + (testing "handle ChronoLocalDateTime" + (is (= {:global {:distinct-count 2 + :nil% 0.0} + :type {:type/DateTime {:earliest "2013-01-01T20:04:00Z" + :latest "2018-01-01T04:04:00Z"}}} + (transduce identity + (fingerprinter (field/map->FieldInstance {:base_type :type/Temporal})) + [(java.time.LocalDateTime/of 2013 01 01 20 04 0 0) + (java.time.LocalDateTime/of 2018 01 01 04 04 0 0)])))) + (testing "handle comparing explicit Instant with ChronoLocalDateTime" + (is (= {:global {:distinct-count 2 + :nil% 0.0} + :type {:type/DateTime {:earliest "2007-12-03T10:15:30Z" + :latest "2018-01-01T04:04:00Z"}}} + (transduce identity + (fingerprinter (field/map->FieldInstance {:base_type :type/Temporal})) + [(java.time.Instant/parse "2007-12-03T10:15:30.00Z") + (java.time.LocalDateTime/of 2018 01 01 04 04 0 0)])))) + (testing "mixing numbers and strings" + (is (= {:global {:distinct-count 2 + :nil% 0.0} + :type {:type/DateTime {:earliest "1970-01-01T00:00:01.234Z" + :latest "2007-12-03T10:15:30Z"}}} + (transduce identity + (fingerprinter (field/map->FieldInstance {:base_type :type/Temporal})) + ["2007-12-03T10:15:30.00Z" 1234])))) + (testing "nil temporal values" + (is (= {:global {:distinct-count 1 + :nil% 1.0} + :type {:type/DateTime {:earliest nil + :latest nil}}} + (transduce identity + (fingerprinter (field/map->FieldInstance {:base_type :type/DateTime})) + (repeat 10 nil))))))))))) (deftest disambiguate-test (testing "We should correctly disambiguate multiple competing multimethods (DateTime and FK in this case)" diff --git a/test/metabase/test.clj b/test/metabase/test.clj index 8e597be57798a..d8a7f411ed341 100644 --- a/test/metabase/test.clj +++ b/test/metabase/test.clj @@ -299,7 +299,9 @@ (with-temp* [toucan-model [x] toucan-model [y]] (let [[_ _ things-in-both] (clojure.data/diff x y)] - things-in-both)))) + ;; don't include created_at/updated_at even if they're the exactly the same, as might be the case with MySQL + ;; TIMESTAMP columns (which only have second resolution by default) + (dissoc things-in-both :created_at :updated_at))))) (fn [toucan-model] (initialize/initialize-if-needed! :db) (db/resolve-model toucan-model)))) diff --git a/test/metabase/test/util.clj b/test/metabase/test/util.clj index 7d39560773a6a..cf259fee03ff0 100644 --- a/test/metabase/test/util.clj +++ b/test/metabase/test/util.clj @@ -114,123 +114,154 @@ (defn- rasta-id [] (user-id :rasta)) -(u/strict-extend (class Card) - tt/WithTempDefaults - {:with-temp-defaults (fn [_] {:creator_id (rasta-id) - :dataset_query {} - :display :table - :name (random-name) - :visualization_settings {}})}) - -(u/strict-extend (class Collection) - tt/WithTempDefaults - {:with-temp-defaults (fn [_] {:name (random-name) - :color "#ABCDEF"})}) - -(u/strict-extend (class Dashboard) - tt/WithTempDefaults - {:with-temp-defaults (fn [_] {:creator_id (rasta-id) - :name (random-name)})}) - -(u/strict-extend (class DashboardCardSeries) - tt/WithTempDefaults - {:with-temp-defaults (constantly {:position 0})}) - -(u/strict-extend (class Database) - tt/WithTempDefaults - {:with-temp-defaults (fn [_] {:details {} - :engine :h2 - :is_sample false - :name (random-name)})}) - -(u/strict-extend (class Dimension) - tt/WithTempDefaults - {:with-temp-defaults (fn [_] {:name (random-name) - :type "internal"})}) - -(u/strict-extend (class Field) - tt/WithTempDefaults - {:with-temp-defaults (fn [_] {:database_type "VARCHAR" - :base_type :type/Text - :name (random-name) - :position 1 - :table_id (data/id :checkins)})}) - -(u/strict-extend (class Metric) - tt/WithTempDefaults - {:with-temp-defaults (fn [_] {:creator_id (rasta-id) - :definition {} - :description "Lookin' for a blueberry" - :name "Toucans in the rainforest" - :table_id (data/id :checkins)})}) - -(u/strict-extend (class NativeQuerySnippet) - tt/WithTempDefaults - {:with-temp-defaults (fn [_] {:creator_id (user-id :crowberto) - :name (random-name) - :content "1 = 1"})}) - -(u/strict-extend (class PermissionsGroup) - tt/WithTempDefaults - {:with-temp-defaults (fn [_] {:name (random-name)})}) - -(u/strict-extend (class Pulse) - tt/WithTempDefaults - {:with-temp-defaults (fn [_] {:creator_id (rasta-id) - :name (random-name)})}) - -(u/strict-extend (class PulseCard) - tt/WithTempDefaults - {:with-temp-defaults (fn [_] {:position 0 - :include_csv false - :include_xls false})}) - -(u/strict-extend (class PulseChannel) - tt/WithTempDefaults - {:with-temp-defaults (constantly {:channel_type :email - :details {} - :schedule_type :daily - :schedule_hour 15})}) - -(u/strict-extend (class Revision) - tt/WithTempDefaults - {:with-temp-defaults (fn [_] {:user_id (rasta-id) - :is_creation false - :is_reversion false})}) - -(u/strict-extend (class Segment) - tt/WithTempDefaults - {:with-temp-defaults (fn [_] {:creator_id (rasta-id) - :definition {} - :description "Lookin' for a blueberry" - :name "Toucans in the rainforest" - :table_id (data/id :checkins)})}) - -;; TODO - `with-temp` doesn't return `Sessions`, probably because their ID is a string? - -(u/strict-extend (class Table) - tt/WithTempDefaults - {:with-temp-defaults (fn [_] {:db_id (data/id) - :active true - :name (random-name)})}) - -(u/strict-extend (class TaskHistory) - tt/WithTempDefaults - {:with-temp-defaults (fn [_] - (let [started (t/zoned-date-time) - ended (t/plus started (t/millis 10))] - {:db_id (data/id) - :task (random-name) - :started_at started - :ended_at ended - :duration (.toMillis (t/duration started ended))}))}) - -(u/strict-extend (class User) - tt/WithTempDefaults - {:with-temp-defaults (fn [_] {:first_name (random-name) - :last_name (random-name) - :email (random-email) - :password (random-name)})}) +(defn- set-with-temp-defaults! [] + (extend (class Card) + tt/WithTempDefaults + {:with-temp-defaults (fn [_] {:creator_id (rasta-id) + :dataset_query {} + :display :table + :name (random-name) + :visualization_settings {}})}) + + (extend (class Collection) + tt/WithTempDefaults + {:with-temp-defaults (fn [_] {:name (random-name) + :color "#ABCDEF"})}) + + (extend (class Dashboard) + tt/WithTempDefaults + {:with-temp-defaults (fn [_] {:creator_id (rasta-id) + :name (random-name)})}) + + (extend (class DashboardCardSeries) + tt/WithTempDefaults + {:with-temp-defaults (constantly {:position 0})}) + + (extend (class Database) + tt/WithTempDefaults + {:with-temp-defaults (fn [_] {:details {} + :engine :h2 + :is_sample false + :name (random-name)})}) + + (extend (class Dimension) + tt/WithTempDefaults + {:with-temp-defaults (fn [_] {:name (random-name) + :type "internal"})}) + + (extend (class Field) + tt/WithTempDefaults + {:with-temp-defaults (fn [_] {:database_type "VARCHAR" + :base_type :type/Text + :name (random-name) + :position 1 + :table_id (data/id :checkins)})}) + + (extend (class Metric) + tt/WithTempDefaults + {:with-temp-defaults (fn [_] {:creator_id (rasta-id) + :definition {} + :description "Lookin' for a blueberry" + :name "Toucans in the rainforest" + :table_id (data/id :checkins)})}) + + (extend (class NativeQuerySnippet) + tt/WithTempDefaults + {:with-temp-defaults (fn [_] {:creator_id (user-id :crowberto) + :name (random-name) + :content "1 = 1"})}) + + (extend (class PermissionsGroup) + tt/WithTempDefaults + {:with-temp-defaults (fn [_] {:name (random-name)})}) + + (extend (class Pulse) + tt/WithTempDefaults + {:with-temp-defaults (fn [_] {:creator_id (rasta-id) + :name (random-name)})}) + + (extend (class PulseCard) + tt/WithTempDefaults + {:with-temp-defaults (fn [_] {:position 0 + :include_csv false + :include_xls false})}) + + (extend (class PulseChannel) + tt/WithTempDefaults + {:with-temp-defaults (constantly {:channel_type :email + :details {} + :schedule_type :daily + :schedule_hour 15})}) + + (extend (class Revision) + tt/WithTempDefaults + {:with-temp-defaults (fn [_] {:user_id (rasta-id) + :is_creation false + :is_reversion false})}) + + (extend (class Segment) + tt/WithTempDefaults + {:with-temp-defaults (fn [_] {:creator_id (rasta-id) + :definition {} + :description "Lookin' for a blueberry" + :name "Toucans in the rainforest" + :table_id (data/id :checkins)})}) + + ;; TODO - `with-temp` doesn't return `Sessions`, probably because their ID is a string? + + (extend (class Table) + tt/WithTempDefaults + {:with-temp-defaults (fn [_] {:db_id (data/id) + :active true + :name (random-name)})}) + + (extend (class TaskHistory) + tt/WithTempDefaults + {:with-temp-defaults (fn [_] + (let [started (t/zoned-date-time) + ended (t/plus started (t/millis 10))] + {:db_id (data/id) + :task (random-name) + :started_at started + :ended_at ended + :duration (.toMillis (t/duration started ended))}))}) + + (extend (class User) + tt/WithTempDefaults + {:with-temp-defaults (fn [_] {:first_name (random-name) + :last_name (random-name) + :email (random-email) + :password (random-name)})})) + +(set-with-temp-defaults!) + +;; if any of the models get redefined, reload the `with-temp-defaults` so they apply to the new version of the model +(doseq [model-var [#'Card + #'Collection + #'Dashboard + #'DashboardCardSeries + #'Database + #'Dimension + #'Field + #'Metric + #'NativeQuerySnippet + #'Permissions + #'PermissionsGroup + #'Pulse + #'PulseCard + #'PulseChannel + #'Revision + #'Segment + #'Table + #'TaskHistory + #'User]] + (remove-watch model-var ::reload) + (add-watch + model-var + ::reload + (fn [_ reference _ _] + (println (format "%s changed, reloading with-temp-defaults" model-var)) + #_(set-with-temp-defaults!)))) ;;; ------------------------------------------------- Other Util Fns -------------------------------------------------