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

Activity entries for dashboard-add-cards events using incorrect card permalink #18547

Closed
lukeman opened this issue Oct 19, 2021 · 2 comments · Fixed by #18578
Closed

Activity entries for dashboard-add-cards events using incorrect card permalink #18547

lukeman opened this issue Oct 19, 2021 · 2 comments · Fixed by #18578
Assignees
Labels
.Correctness Organization/Activity Log 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
Milestone

Comments

@lukeman
Copy link

lukeman commented Oct 19, 2021

Describe the bug
When cards are added to a dashboard, the activity entry in the frontend links to the wrong question ID. It's using the id of the report_dashboardcard instead of the card_id that was added to the dashboard.

To Reproduce
Steps to reproduce the behavior:

  1. Add an existing card to a dashboard, observing the question's ID
  2. Click through to Settings > Activity
  3. Observe the link to the card you just added uses an incorrect ID (+ the correct slug for the question)

Expected behavior
Card name in the activity should link to the card as it does when the same card is created or edited.

Screenshots
Note the permalinks for the activity when I created the card and then the second when I added that card to a dashboard:
bug-repro

Activity log entry for the dashboard-add-cards event (both the join table id and the card_id are present):
Screen Shot 2021-10-18 at 8 47 17 PM

Information about your Metabase Installation:

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

  • Your browser and the version: Safari
  • Your operating system: Mac
  • Your databases: Postgres
  • Metabase version: 0.41.0
  • Metabase hosting environment: Self-hosted dockerfile + nomad
  • Metabase internal database: Postgres

Severity
P3. Bug is limited to the activity log and mostly just confused me for a minute.

Additional context
I heart Metabase and all of y'all.

@lukeman lukeman added .Needs Triage Type:Bug Product defects labels Oct 19, 2021
@flamber flamber added .Correctness .Regression Bugs that were previously fixed and/or bugs unintentionally shipped with new features. Organization/Activity Log Priority:P2 Average run of the mill bug and removed .Needs Triage labels Oct 19, 2021
@flamber
Copy link
Contributor

flamber commented Oct 19, 2021

Regression since 0.40.0. Giving P2, since it makes the Activity Log much less useful, but could potentially link to the wrong questions, which could cause misunderstandings.

@nemanjaglumac
Copy link
Member

nemanjaglumac commented Oct 20, 2021

If it helps, the logic behind this bumps the counter for every new added card, instead of respecting the card's id.

To reproduce:

  • add a card to the dashboard - see that it has id 1
  • add the same card to the dashboard again - now it has id 2

@nemanjaglumac nemanjaglumac added this to Backlog in Cypress Testing Oct 20, 2021
nemanjaglumac added a commit that referenced this issue Oct 20, 2021
nemanjaglumac added a commit that referenced this issue Oct 20, 2021
@nemanjaglumac nemanjaglumac added the .Reproduced Issues reproduced in test (usually Cypress) label Oct 20, 2021
github-actions bot pushed a commit that referenced this issue Oct 20, 2021
@nemanjaglumac nemanjaglumac self-assigned this Oct 20, 2021
nemanjaglumac added a commit that referenced this issue Oct 20, 2021
… in the activity page (#18575) (#18576)

Co-authored-by: Nemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com>
nemanjaglumac added a commit that referenced this issue Oct 21, 2021
…e dashboard [Activity log] (#18578)

* Fix wrong id applied when constructing a link to the question added to the dashboard

Closes #18547

* Unskip repro

* Add unit tests for question ids

* Guard against the dashboard questions with no names
@nemanjaglumac nemanjaglumac added this to the 0.42 milestone Oct 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.Correctness Organization/Activity Log 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
Projects
3 participants