-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Add Unarchive button to a dashboard #4697
Conversation
|
||
const unarchiveDashboard = () => { | ||
recordEvent("unarchive", "dashboard", dashboard.id); | ||
updateDashboard({ is_archived: false }, false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gabrieldutra I've noticed for some of the update operations we don't send dashboard version information. When I did try sending version here, it had out of date version. It seems like we're hiding some flawed logic by avoiding sending the version with all updates. For example here: after unarchiving a dashboard you can no longer edit it.
We should address this in a separate PR, but wanted to point out to start the discussion in case I overlooked something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The archive function is the only one that doesn't send dashboard version information, no? Anyway, I think I know what's going on, the update function is set to only update the changed keys from the dashboard
object. Since the archive method doesn't send the version information, it keeps the outdated version.
In case you want to make it work for this PR (or lmk if you want me to push a commit), it should be a matter of changing this line to pick(updatedDashboard, [...keys(data), "version"]
:
setDashboard(currentDashboard => extend({}, currentDashboard, pick(updatedDashboard, keys(data)))) |
Most likely the backend hasn't been updated yet 😅, it's working in my local environment anyway :) |
|
||
const unarchiveDashboard = () => { | ||
recordEvent("unarchive", "dashboard", dashboard.id); | ||
updateDashboard({ is_archived: false }, false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The archive function is the only one that doesn't send dashboard version information, no? Anyway, I think I know what's going on, the update function is set to only update the changed keys from the dashboard
object. Since the archive method doesn't send the version information, it keeps the outdated version.
In case you want to make it work for this PR (or lmk if you want me to push a commit), it should be a matter of changing this line to pick(updatedDashboard, [...keys(data), "version"]
:
setDashboard(currentDashboard => extend({}, currentDashboard, pick(updatedDashboard, keys(data)))) |
i need to know when the Un-archive button ready to be released ? |
Thanks for pinging this @amirashakernady . I'll prioritise this one for this month. |
Thanks a million @susodapop |
0ff1465
to
235d468
Compare
235d468
to
287df5b
Compare
I've rebased off the latest |
Co-authored-by: Jesse Whitehouse <[email protected]>
What type of PR is this? (check all applicable)
Description
Follow up to #4669, added an "Unarchive" button to an archived dashboard.
Related Tickets & Documents
redashlabs/product#47
closes #982
Mobile & Desktop Screenshots/Recordings (if there are UI changes)