Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hiding a table makes it impossible to edit existing questions that uses that table #15947

Open
flamber opened this issue May 6, 2021 · 8 comments
Labels
Administration/Table Metadata .Frontend Organization/Saved Questions Priority:P2 Average run of the mill bug Querying/GUI Query builder catch-all, including simple mode .Reproduced Issues reproduced in test (usually Cypress) .Team/QueryingComponents Type:Bug Product defects .Wanted: MLv2 Issues that will be fixed (or easier to fix, or possible to fix) when we have MLv2

Comments

@flamber
Copy link
Contributor

flamber commented May 6, 2021

Describe the bug
When questions have been built on tables, which are hidden later, then it's either impossible to edit the question or the question will drop all references to the hidden table.

Possible workaround: Change the table name in Admin > Data Model by prepending ~DEPRECATED or similar, so the table is added to the end of tables.

To Reproduce

  1. Custom question > Sample Dataset > create a question like this and save as "Q1":
    image
  2. Admin > Data Model > Sample Dataset > hide Orders table - refresh browser and view "Q1"
    It's now not possible to edit the question at all - almost looks like a user without data access
    image
  3. Admin > Data Model > Sample Dataset > show Orders, but hide Products table - refresh browser and view "Q1"
    It's possible to edit the question, but making any changes will drop all references to Products table
    image

Expected behavior
This issue might not be straight forward, since it should probably allow existing questions to keep working, but should not include all hidden tables in the API response by default, since that could introduce a performance issue on instances, where perhaps 1k tables have been hidden for technical reasons, because it's not possible to limit via the db privileges.
A possible solution would be to introduce a "Deprecated" along with "Technical" and "Cruft", which would then include those tables in the API.

Information about your Metabase Installation:
Tested 0.37.8 thru 0.39.1

Related to #6445 and #6298

@flamber flamber added Type:Bug Product defects Priority:P2 Average run of the mill bug Administration/Table Metadata Querying/GUI Query builder catch-all, including simple mode labels May 6, 2021
nemanjaglumac added a commit that referenced this issue May 7, 2021
@nemanjaglumac nemanjaglumac added this to Backlog in Cypress Testing May 7, 2021
nemanjaglumac added a commit that referenced this issue May 10, 2021
…tions (#15966)

* Add repro for #15947

* Expand the assertion with the joined table

Co-authored-by: flamber <1447303+flamber@users.noreply.github.com>
@nemanjaglumac nemanjaglumac added the .Reproduced Issues reproduced in test (usually Cypress) label May 10, 2021
@ariya ariya self-assigned this Mar 23, 2022
@nemanjaglumac nemanjaglumac removed this from Backlog in Cypress Testing May 4, 2022
@ranquild ranquild added the .Wanted: MLv2 Issues that will be fixed (or easier to fix, or possible to fix) when we have MLv2 label Aug 10, 2023
@likeshumidity
Copy link
Contributor

Should Metabase show a warning or prevent marking a table hidden if it is used in a question?

@krschacht
Copy link

Another possibility would be: once a table is hidden still let the query be shown even if you don't allow editing of it. Or, even more minor, at least let the user know why they can't edit the query. I lost a bunch of cycles on this issue so 👍 to getting it fixed.

@nemanjaglumac
Copy link
Member

nemanjaglumac commented Sep 29, 2023

@krschacht upvote the original post. We look at the number of those upvotes.
Upvote in a comment will get lost.

@krschacht
Copy link

@nemanjaglumac I don't see an option to upvote anywhere on this issues page. Maybe that's only for people who are part of the Metabase organization?

@nemanjaglumac
Copy link
Member

@krschacht the left bottom of the first post
image

@nemanjaglumac nemanjaglumac self-assigned this Nov 8, 2023
@nemanjaglumac
Copy link
Member

Still able to reproduce this on latest master at d35c779
The existing E2E reproductions seems to be valid.

Please note that converting a question into a model shows "Filter" and "Summarize", and converting it back to the saved question again removes them.

@bshepherdson
Copy link
Contributor

I think this should remain with the QC team and be triaged by them. There's only a couple of basic conversions of the visibility_state fields on tables or fields in MLv2; I think the logic for eg. not allowing editing of a question based on a hidden table belongs to the FE components.

@kamilmielnik
Copy link
Contributor

Reproducible in master at 78cb288.

If the fix is to disallow editing queries referencing hidden tables, then it's QC area. I'll change label from BE to FE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Administration/Table Metadata .Frontend Organization/Saved Questions Priority:P2 Average run of the mill bug Querying/GUI Query builder catch-all, including simple mode .Reproduced Issues reproduced in test (usually Cypress) .Team/QueryingComponents Type:Bug Product defects .Wanted: MLv2 Issues that will be fixed (or easier to fix, or possible to fix) when we have MLv2
Projects
None yet
Development

No branches or pull requests