Form Screen Templates Generate Relevant Row Ids, Permission Roles are Automatically Generated

[Ghrehh]

ticket

Main Changes

• Form Screen "View" and "Edit" templates now include the relevant parameters to pull an id out of the page url, like resource/view/:id. Previously they did not, which meant they weren't super useful out of the box.
• Screen templates no longer prompt the user to select an access role, instead one will be selected based on the datasource used to create the template, or in the case of a blank screen, it will be set to "Basic".

Other Changes

• Nav Links will no longer be automatically added for generated "View" and "Edit" form screens, they require ids in their url to work, so this behaviour didn't make much sense.
• Refactored the functions that generate screens to have a more unified interface, also removed some unused stuff.
• Heavy refactoring of the CreateScreenModal component. Its logic was a little confusing in my opinion, and it also had a lot of code that seemed to be holdovers of previous features.
• Selecting and unselecting datasources during screen creation now works correctly. Previously, if you had multiple datasources selected, then clicked an already selected one, it would deselect every other datasource, which is the inverse of the expected behaviour
• Previously selected datasources are now remembered whenever a user hits the back button on the 'Select Form Type' part of the form template flow.
• Permissions are prefetched whenever a datasource is selected, as they need to be request individually per resource, the modal will then resolve this promises whenever it creates the screens and needs access to that resource's permissions. At this point, this requests should already have completed, but if not the modal will wait until then.

Table Delete Modal

This PR also contains some changes for the Table Delete Warning modal.

Previously this modal would delete any screens generated with the table when the table was deleted, this seems a little extreme given that the screen may contain other data, or may not even contain the originally generated content at all. There was also no way to remove this tagging as far as I can tell, so a screen was stuck with it forever.

It also was inconsistently applied, and screens generated from form templates weren't marked in the same way.

Now this modal will simply warn the user that a screen was generated from this datasource initially, and that it may be affected by table deletion. This behaviour is also consistently applied across table and form templates.

Description	Image
base
views
views, screens, overflow
many screens	)

Other Changes

• The table name no longer overflows
• The screen names no longer overflow
• Different layout
• Screen names now open the screen in the design section, in a new tab.
• Now mentions if the table has views or not and gives a count.
• Added the "click the table name to populate the name field" helper we have in other delete modals to this one.

[deanhannigan]

Yo @Ghrehh! Overall a really great addition 🎉 It definitely feels more streamlined creating screens now.
Some minor comments in the review and a couple bugs I ran into during testing.

• BUG Error previewing newly created View/Update screens from External Table/View
  • Get the following notification invalid input syntax for type smallint: " :id "
  • This was from a Postgres external integration.
  • This doesn't occur in prod for internal or external.
  • The default builder behaviour is to load the first record, the preview should reflect that
• BUG caching view permissions
  • Inherited Permissions used to generate view screens don't update when the parent table access is updated.
  • Custom view access permissions also don't seem to have any impact on the screen generated, in this state.
  • Create a table and set the following access permissions
    • Read/POWER, Write/PUBLIC
  • Create a view and leave it unchanged. It will inherit the above.
  • Generate a screen from the new view and it will have permissions that reflect the parent table.
    • Read/POWER, Write/PUBLIC
  • Alter the permissions of the parent table to something completely different
    • e.g Read/BASIC, Write/POWER
  • Generate a new screen from the same view without touching permissions.
    • It will generate a screen reflecting the parent tables original permissions.
    • Altering the custom permissions of the view doesn't have any effect.

Notes - while debugging the permissions issue above I thought it would be nice to give the user an impression of what the computed screen permission would be before they create it. We used to prompt them with the default, now we set it for them and don't indicate it to them. Below is a PoC:

[deanhannigan]

NAB: We should probably reflect the same confirmation UX here as we do for tables within datasources. This is obviously a much scarier thing to do.

Also, I realise you didn't write this, but it might be nice to add here 👍

[Ghrehh]

Could you explain this again please? I'm not sure I follow 😆

[deanhannigan]

No worries. I just noticed that when running through the PR, when you go to delete an entire datasource from BB, we don't provide the same confirmation mechanism as we do for a table within a datasource. For tables, we provide a text prompt and require the user to enter the exact table name before enabling the delete button.

When deleting my entire PostgreSQL source, I only get this:

Utterly outside the scope of the ticket though. Safe to ignore and I'll just raise a ticket for it 👍

[aptkingston]

Really nice work Gerard! Ran through this and everything worked as expected. I like that you've accounted for small details like not adding nav links for edit/view forms etc 👌

Please feel free to give me a ping if you're doing new features like adding the read/write permissions in to the datasource modal - I'd always want to weigh in with opinions from a product/design perspective for unspecced changes 👍 Obviously with Ben on board now he can be pulled into these sorts of conversations too, since they have a visual impact.

But great job and I couldn't find any issues here at all.

[Ghrehh]

@deanhannigan Ty for the thorough review, responses and changes below, if you could review this again when you get a chance I'd appreciate it:

BUG Error previewing newly created View/Update screens from External Table/View

I think this is happening on master currently and isn't to do with my changes

BUG caching view permissions

I think a lot, if not all, of the stuff you're mentioning here is caused by the fact that the original implementation of this just fetched definitions once and didn't bother to refetch them. I thought this would be okay to begin with, but I think it's a mistake now.

I've changed this so that the caching takes place at the component level, and unselecting a datasource and reselecting it will refetch the role data. Opening and closing the screen creation modal will blow away all cached values entirely also.

while debugging the permissions issue above I thought it would be nice to give the user an impression of what the computed screen permission would be...

I also implemented this! But It's a little more complicated because both read and write access levels need to be exposed to the user, I'll include a screenshot and a full change log below.

NOTE: Upon talking with Cheeks I think we might actually remove this, as the original intention of this automatic role assignment change was to make it so users didn't need to consider screen roles as much, and displaying them even subtly kind of goes against this philosophy. I'll leave it in this PR for now just to get it over the line with minimal changes, but will remove it in #14203 which is branched off this.

Changes

• Made a change so that long table names don't overflow the datasource selection modal.
• Added some error handling to the requests for permissions/role information.
• Did some refactoring of the datasource modal component generally.
• Changed the datasource modal selected state to be a blue border instead of a checkmark, as this is more consistent with the never table type screen introduced in [BUDI-8460] Add New Step to Table Screen Template #14203.
• Added read/write role indicators to the datasources in the datasource selection modal, but as mentioned above, this will be removed in a followup PR.

[deanhannigan]

LGTM! I ran through everything this morning and the changes are looking great! I had a chat with @aptkingston this morning about the preview issue on master and I'm going to raise a ticket for it.

I love the permissions indicators on the datasource picker! I do see now how this conflicts with the intended design philosophy though 👍