fix(dashboard-list): change name of dashboard is not reflected instantly #15186
Conversation
Codecov Report
@@ Coverage Diff @@
## master #15186 +/- ##
==========================================
- Coverage 76.88% 76.87% -0.01%
==========================================
Files 976 976
Lines 51318 51322 +4
Branches 6907 6910 +3
==========================================
Hits 39455 39455
- Misses 11644 11648 +4
Partials 219 219
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Could we try to use the response from the request we're already making? There's the updated title available: After your change, we make additional 2 request, which seems like a waste |
Cool. It seems that we have updated the data using the response, but the list view is not updated. I will look at why there is no update. |
| @@ -157,7 +157,7 @@ function DashboardList(props: DashboardListProps) { | |||
| ({ json = {} }) => { | |||
| setDashboards( | |||
| dashboards.map(dashboard => { | |||
| if (dashboard.id === json.id) { | |||
| if (dashboard.id === json?.result.id) { | |||
There was a problem hiding this comment.
Nice 😄 Can we add 1 more null check, just to be safe? json?.result?.id
There was a problem hiding this comment.
Sure! But there will be another problem, here I only need to update the title, colorSchema, json_metadata, etc., but the modified time is controlled by changed_on_delta_humanized, the response does not have this field.
There was a problem hiding this comment.
Can we just add that param to the response in backend? @zhaoyongjie @villebro
There was a problem hiding this comment.
@kgabryje makes sense - I'll see if it's easy to add it.
There was a problem hiding this comment.
Thanks! BTW, If we update the dashboard's title, since the list is sorted by modified_time or modified_by, not only update the title, the order of the list also needs to be updated. This may be a case I need to consider. So I think refreshData can simplify a lot, but it will add additional requests.
There was a problem hiding this comment.
I have added changed_on_delta_humanized field in the RESTful response.
link to: #15542
There was a problem hiding this comment.
I think changing the order of the list will be surprising for the user. It may look like the dashboard they were editing has disappeared.
stephenLYZ
force-pushed
the
fix-dashboard-name
branch
from
2b5b600
to
10f8acf
Compare
stephenLYZ
force-pushed
the
fix-dashboard-name
branch
from
10f8acf
to
32f3f68
Compare
|
/testenv up |
|
@junlincc Ephemeral environment spinning up at http://54.218.174.21:8080. Credentials are |
|
Ephemeral environment shutdown and build artifacts deleted. |
…tly (apache#15186) * fix: change name of dashboard is not reflected instantly * fix: id * fix: update info * fix: add changed_on_delta_humanized
…tly (apache#15186) * fix: change name of dashboard is not reflected instantly * fix: id * fix: update info * fix: add changed_on_delta_humanized
…tly (apache#15186) * fix: change name of dashboard is not reflected instantly * fix: id * fix: update info * fix: add changed_on_delta_humanized
mistercrunch
added
the
🏷️ bot
SUMMARY
Fix the title of dashboard list is not reflected instantly after editing.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
before
2021-06-16.1.02.49.mov
after
2021-06-16.1.03.35.mov
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION