Stop Automatically Enabling Columns Added to a Table in its Views

[Ghrehh]

trafficstars

Noticed an issue/oversight where adding a column to a table with views would also automatically enable that column on the view. This not only seems a little insecure, but also wasn't reflected in the UI, which recognised the newly added column as disabled.

Though the field is marked disabled in the UI, a consumer of the view in the client has access to the new field, which can be seen by looking at the API response or doing some binding trickery.

This seems to be due to how columns being enabled or disabled in views is handled. For the backend, the presence of the column in the view schema denotes that it's enabled, and visible: true/false isn't respected. The code that was removed to fix the issue below actually enabled the field, which doesn't seem to be the intention.

I don't think this fix breaks anything else, but it would be good for someone on the backend team to look over this for me please 🙏 The code for views was very complex 🙃

[samwho]

This appears to have broken some tests, and I'm not sure myself what the correct behaviour would be. @mike12345567?

[mike12345567]

This appears to have broken some tests, and I'm not sure myself what the correct behaviour would be. @mike12345567?

This is due to the way the tests are written - they depended on the syncSchema function which has been adjusted in this PR adding the columns to the view.

This functionality changes that logic so that columns are not added to views without a specific call to do so - therefore the tests will need to be updated to mimic this logic.

Everywhere in the views.spec.ts test that the syncSchema has been called, this will need to be adjusted to instead explicitly sync the schema and then update the schema with the columns. This is a more unit-test-y example of a test as it doesn't go through the API at all, which makes this a little more difficult.

This is a tough one because the test does appear to dependent on the functionality that this is changing, so the test does need a bit of reconfiguration to work in the flow that is now expected.

[aptkingston]

Just to check - was the issue that you can see the field in the view schema, or that you actually get access to data in that field if you read a row through the view?

If it's just the former then I'm pretty sure that's by design - the schema isn't intended to be secret, and we want to be able to tell what fields exist but are invisible so we can do things like show them for the builder to toggle visible again, or warn users that there are hidden required fields (which is something the grid currently does).

If it's the latter then yea this definitely needs addressed.

[Ghrehh]

@aptkingston It was the latter, in the scenario I outlined in the main body of the PR a view could expose a value present in the base table that wasn't specified as a column in the view.

[adrinr]

@Ghrehh I could not replicate the issue in master. I created a view, added a new column, but it's hidden in the views for both client and builder. Could you send a video or a step by step replication?

[Ghrehh]

@adrinr Apologies for not having detailed reproduction steps in the original PR message, I hope this helps demonstrate the issue. Sorry for only getting around to this now:

• use master branch
• Create a fresh app.
• Create a BB table like this:

data

|name|value|
|---|---|
|first|1|
|second|2|
|third|3|

• Create a view of that table with all columns enabled.
• Add another column to the original table

dataView

|name|value|valueHidden|
|---|---|---|
|first|1|hidden1|
|second|2|hidden2|
|third|3|hidden3|

• The UI for the view now looks like this:
• Note that the valueHidden column added to the table after the view was created is marked as restricted in the UI, this is IMO the correct and expected behaviour, but as we'll see the backend doesn't behave this way.
• Create a new blank screen in the design section to display the data, using a structure like so:
• Swap the data property of the Repeater Block to point to the view instead of the table.
• Data for the newly added column, that the view should not have access to, is still being displayed.
• This does not seem to be some builder quirk either, publishing the app and viewing the component there will have the same results. Looking at the network tab you will see that the request for the view data is indeed returning the new valueHidden column
• As pointed out in the original PR message, this seems to be because of different methods and backend and frontend use to determine if a field is disabled in a view. The backend seems to check for the existence of a key in the view schema, and the frontend actually checks if the field is marked with visibility: true|false.

Let me know if there's anything else I can do to explain the issue.

[shogunpurple]

@Ghrehh this has been open for over 3 weeks. If we aren't actively working on it can we move it to draft or close?

[Ghrehh]

@shogunpurple Just waiting on some input from the backend folks on if I'm seeing ghosts or if this is a real issue, I'll move it to draft for now and ping them directly on discord.

Edit: Scratch that, will close it.