[connectors] Make cluster/database and datasource/table read-only in views

[john-bodley]

In the CRUD views the cluster/database and datasource/table are required in the forms to ensure that either an added or edited entity is correctly assigned to the parent. This value is normally left blank however it does provide the ability for the user to re-assign (possibly without intent) an entity to another parent which is undesirable.

@mistercrunch ideally it would be great to simply hide these fields from the views, however this doesn't seem to be possible. The only viable solution I am aware of is simply to make the fields read-only per the suggestion outlined here.

Edit

Add

Note for adding we allow blanks otherwise the default displayed value is ill-defined entity even though the entity is correctly parented.

to: @graceguo-supercat @michellethomas @mistercrunch @xtinec

[codecov-io]

Codecov Report

Merging #7073 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master   #7073      +/-   ##
=========================================
+ Coverage   64.48%   64.5%   +0.02%     
=========================================
  Files         421     421              
  Lines       20547   20561      +14     
  Branches     2250    2250              
=========================================
+ Hits        13249   13263      +14     
  Misses       7171    7171              
  Partials      127     127

Impacted Files	Coverage Δ
superset/connectors/druid/views.py	67.74% <100%> (+1.52%)	⬆️
superset/connectors/sqla/views.py	65.28% <100%> (+2.13%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30f88ca...ae13516. Read the comment docs.

[mistercrunch]

I struggled with this in the past, it seemed like an odd FAB requirement/bug. @dpgaspar may have some input/pointers as to how to fix this in FAB.

[dpgaspar]

Hi @john-bodley, @mistercrunch

This seems to be nicely done. The readonly field gives some context to the users, and prevents mistakes.

I've removed the database field from the edit fields on TableModelView without problems.

The problem is with the related view CompactCRUDMixin on master detail (Table->Metrics or Table-Columns), showing a blank on the table field. This is working fine for the edit but seems strange when adding, right?

[mistercrunch]

Personally all I remember is issues with the Inline elements on add and/or edit when the "parent" field is missing. In the example above it would be metrics and columns add/edit columns need to have the table field or they won't get attach to the table in their context.

Knowing it's an Inline, we probably should not even allow to alter the link to the parent. Maybe the form that's built needs some sort of HiddenField for table_id, and FAB should inject it somehow in the context of an inline. [this may or may not be the path to solution]

Keep in mind now that I think those are the last FAB Inline used in Superset, and we're moving away from this particular form as I've re-written the TableEditor in React.

Maybe the real action item here is deprecating the Superset CRUD view for Tables/DruidDatasource. For that, we need to build our own CRUD list view in place of the FAB one.

[dpgaspar]

Yes, probably the best plan of action. My two cents would be to implement it using React components backed by the "new" CRUD REST API.

[john-bodley]

@dpgaspar @mistercrunch is there any additional changes you are requesting from this PR?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

[mistercrunch]

LGTM