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

View Detail on Foreign Key does not work when targeting an item outside the first 2000 rows #21756

Closed
iethree opened this issue Apr 19, 2022 · 5 comments · Fixed by #22140
Closed
Assignees
Labels
.Correctness Difficulty:Hard .Frontend Priority:P2 Average run of the mill bug .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. .Reproduced Issues reproduced in test (usually Cypress) Type:Bug Product defects Visualization/Tables Raw, summarized, and pivoted tables
Milestone

Comments

@iethree
Copy link
Contributor

iethree commented Apr 19, 2022

Describe the bug
When clicking "view detail" on a foreign key - if it is not in the first 2000 rows of the table, it will give the user a not found message.

Because of the nature of the bug, I don't think it's reproducible with the sample DB.

Logs

(nothing material)

To Reproduce
Steps to reproduce the behavior:

  1. Go to a table with more than 2000 foreign keys defined
  2. Click on a foreign key (that's not in the first 2000 rows of the table) and select "View Details"
  3. Instead of seeing the object's details, you will get a "we're a little lost" error

Expected behavior
The user should see the detail view of the object selected

Screenshots

detailNotFound.mp4

Information about your Metabase Installation:

You can get this information by going to Admin -> Troubleshooting.

  • Your browser and the version: Google Chrome Version 100.0.4896.127 (Official Build) (64-bit)
  • Your operating system: Ubutntu 20.04
  • Your databases: Postgres 14.2
  • Metabase version: master branch - 2022-04-19 - v0.42.1-SNAPSHOT"
  • Metabase hosting environment: local development
  • Metabase internal database: H2

Severity

Makes foreign-key object detail view useless for most large tables.

@iethree iethree added Type:Bug Product defects .Needs Triage labels Apr 19, 2022
@flamber flamber added Priority:P2 Average run of the mill bug .Correctness .Frontend .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. and removed .Needs Triage labels Apr 22, 2022
@flamber flamber added this to the 0.43 milestone Apr 22, 2022
@flamber
Copy link
Contributor

flamber commented Apr 22, 2022

Regression in 0.43.0-rc1 since #20101 @kulyk

P1 contender.

Reproduce 1 - problematic

  1. Question > Sample > Orders
  2. Filter ID - input 2 and 18000
    image
  3. Click User_ID=2392 > View details
    image
  4. We're a little lost...
    image

Reproduce 2 - less problematic

  1. Question > Sample > Orders - save as "Q1"
  2. Add to dashboard and create Dropdown filter connected to Products.Category
  3. Filter by Doohickey and click question title
    image
  4. On the question, it shows a table with everything correct - and clicking ID=6 will show "Previously" correctly disabled
    image
  5. Go back to the dashboard and filter by Doohickey and click ID=6
    image
  6. Show Object Details with "Previous" enabled, which is incorrect
    image

@flamber flamber added the Visualization/Tables Raw, summarized, and pivoted tables label Apr 22, 2022
@ranquild ranquild self-assigned this Apr 26, 2022
@ranquild
Copy link
Contributor

ranquild commented Apr 26, 2022

After spending a few hours on trying different approaches I believe that the correct way to fix this is to refactor the detailed view to load data for the selected entity and DO NOT use data from the active query. This would allow opening the view within a query for related entities without loading another table which might actually not have the selected entity.

@mazameli
Copy link
Contributor

@ranquild I'm not sure how to unblock you. Questions:

  1. What is the user-facing/UX consequenc(es) of "refactor the detailed view to load data for the selected entity and DO NOT use data from the active query", if any?
  2. How long do you think this proposed refactor would take? Does @flamber et al think this needs to block 0.43?
  3. Does @kulyk have any thoughts that could help?

@ranquild
Copy link
Contributor

@mazameli If we change the object details view in this way we would not navigate to a different table when browsing details. We could make Up and Down buttons work by navigating between results in the current dataset.

@ranquild
Copy link
Contributor

fyi @kulyk @flamber @mazameli an easy fix would be to add a primary key filter when navigating to another table. E.g. when clicking on User ID in Orders table we currently open People without filters and fail when there is no selected entity. But we could open the table with a filter for the primary key and it won't fail.

@ranquild ranquild assigned ranquild and kulyk and unassigned ranquild Apr 27, 2022
@ranquild ranquild removed their assignment Apr 27, 2022
kulyk added a commit that referenced this issue Apr 28, 2022
kulyk added a commit that referenced this issue Apr 28, 2022
* Fix FK filter

* Add query

* Use `question` click object for FK drills

* Add null check to `loadObjectDetailFKReferences`

* Fix `ObjectDetailDrill` unit test

* Accept explicit `objectId` in FK redux actions

* Fix type

* Fix `objectId` type

* Fix opening ObjectDetail for FK drill

* Fix loading FK references

* Fix following FKs

* Hide prev/next buttons when drilling FKs

* Fix expected URL after FK drill in tests

* Use object detail test helpers

* Reproduce #21756

* Fix dashboard drill test

Co-authored-by: Alexander Polyankin <alexander.polyankin@metabase.com>
kulyk added a commit that referenced this issue Apr 28, 2022
* Fix FK filter

* Add query

* Use `question` click object for FK drills

* Add null check to `loadObjectDetailFKReferences`

* Fix `ObjectDetailDrill` unit test

* Accept explicit `objectId` in FK redux actions

* Fix type

* Fix `objectId` type

* Fix opening ObjectDetail for FK drill

* Fix loading FK references

* Fix following FKs

* Hide prev/next buttons when drilling FKs

* Fix expected URL after FK drill in tests

* Use object detail test helpers

* Reproduce #21756

* Fix dashboard drill test

Co-authored-by: Alexander Polyankin <alexander.polyankin@metabase.com>
This was referenced Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.Correctness Difficulty:Hard .Frontend Priority:P2 Average run of the mill bug .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. .Reproduced Issues reproduced in test (usually Cypress) Type:Bug Product defects Visualization/Tables Raw, summarized, and pivoted tables
Projects
None yet
6 participants