[GitHub] williaster commented on issue #4791: apply new Dashboard builder to dashboard

[GitBox <gi...@apache.org>]

williaster commented on issue #4791: apply new Dashboard builder to dashboard
URL: https://github.com/apache/incubator-superset/pull/4791#issuecomment-380619142
 
 
   @graceguo-supercat 
   
   1) the code history etc is a valid concern. I think we should still delete v1, but move files with `git mv ...` so that it preserves history etc. otherwise we won't be sure we've deleted all unused code esp css.
   
   2) thanks for this explanation. next step is to clean it up / align on what it should be going forward! I think this would be an improvement:
   
   ```
   datasources
   toastMessages
   chartMetadata
   chartData
   dashboardLayout
   dashboardMetadata
       title
       standalone_mode
       slug
       metadata
       id
       edit_perm  
       save_perm
       sliceIds => (delete this?)
   dashboardState
       filters
       userId
       editMode
       hasUnsavedChanges
       showBuilderPane
       common
   ```
   
   **Explicit changes**
   - `allSlices` => `chartMetadata` (could consider `componentMetadata`, in the future we will have more than just "slices"?)
   - `charts` => `chartData`
   - `state.dashboard.dashboard` => separate it out to `state.dashboardMetadata`, it shouldn't be mixed with current UI state like filters/edit mode. `dashboard.dashboard` is not readable.
   - `state.dashboard.[rest]` => rename this state tree `dashboardState`
    
   I also don't want this point to be lost in the comments:
   We shouldn't set the id for `chartData` to `slice_[id]`, it should just be `id` and match `chartMetaData`. Similarly we should remove the idea of `chartKey` which you use, just use id/we shouldn't over complicate with different words for the same thing.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services