Implement File column configuration

[pavish]

Fixes #4763

Checklist

• My pull request has a descriptive title (not a vague title like Update index.md).
• My pull request targets the develop branch of the repository
• My commit messages follow best practices.
• My code follows the established code style of the repository.
• I added tests for the changes I made (if applicable).
• I added or updated documentation (if applicable).
• I tried running the project locally and verified that there are no
  visible errors.

Developer Certificate of Origin

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

[pavish]

@seancolsen @zackkrida I wanted to let you both know the direction I'm heading with this PR.

There will be a UI type for "File" displayed to the user in the "Add column" dropdown as well as the "Change data type" dropdown. As far as the user experience is concerned, the "File" type works like every other type displayed in the type selection menu.

Add Column	Update Data type

In other parts of the product where column metadata is not yet accessible, this column would be recongized as a jsonb column.

Data Explorer

Now, this is not entirely functional yet, but I've made essential modifications towards getting this UX working. The work to send the request to update column metadata while creating/updating is pending.

More relevant to @seancolsen: This affects work in the other Files PRs. Essentially, only the lower levels of the frontend has to be aware of files and all higher levels would have to treat File like any other type, instead of treating it like a PK/FK. The changes are best demonstrated by this commit: 08ef68d.

[zackkrida]

That sounds great, @pavish — it feels like the "correct" way to solve this. I'll keep an eye out for any concerns or more detailed feedback from @seancolsen .

[seancolsen]

This works great @pavish!

I think if I had chosen this UI approach, it probably would have taken me like 3x the time it took you, just since it touches this abstractType system that you're much more familiar with.

On our call you said that your changes are still pretty ugly and you might want to spend another day or so cleaning it up. But I think we can to that later. For now, I can see that this works. So I'm marking it as ready to merge.

I'm holding off on merging it though, just to make it easer for you to review my PR #4758.

I did a quick skim over the diff, but I didn't think too deeply about it, just because this code badly needs to be refactored anyway so I'm not too concerned about adding additional tech debt in this area of the codebase right now.

The linting errors here seem to be from changes in the base branch. We can sort that out independently of this PR.

[seancolsen]

This is ready to merge once the base branch becomes file_attachment_feature.

[pavish]

@seancolsen I notice you've reviewed this PR before I had marked it as ready.

It still doesn't handle case where the 'Files' option should be disabled or hidden when commonData does not contain file_backends, and it also hardcodes this value as default in multiple places. This will break when the user does not have file backends configured. There might be other bugs that I've not ironed out yet.

I'm going to hold off on merging this in, until I fix that.

[seancolsen]

@pavish it seems we had a miscommunication.

Based on our call I thought you were done here. Specifically:

• At 33:34 you said:

  I can review yours, get it merged. And you can review mine, get it merged. And then we can just sort of do all the rest of the work.

• At 34:37 I said:

  It sounds like what we should do is prioritize reviewing my PR and your PR and getting them both merged into file_attachment_feature. And that at that point, you and I can base our PRs on top of file_attachment_feature. That's our first order of business.

• At 35:25 you said:

  I'll focus on reviewing your PR first, and then I'll start to work on these [items in the checklist]

  This point in particular gave me the impression that you were not planning on putting more work into this PR — but rather that you would sort out remaining tasks in subsequent PRs.

Even with the miscommunication though, I'm not sure what the harm would be in merging this. That's why I'm biasing my actions towards moving forward (quickly!) here, rather than waiting, double-checking with you to make sure we have a green light. Is there a problem with merging it as-is?

My problem with keeping it open is that we have a lot of other work stacked on this. And I want to be able to merge that work so that we can avoid merge conflicts now that we're spreading these small tasks across three devs.

[pavish]

Is there a problem with merging it as-is?

This PR touches a lot of files and I wasn't confident that these changes wouldn't break anything else. I didn't want to rush this PR since I had not completed due diligence on my end. That's why the PR wasn't marked as ready to review.

sort out remaining tasks in subsequent PRs.

You're right. I'll get the changes mentioned in this comment in subsequent PRs.

I took some time to test all affected areas and added one minimal commit. With this, this PR can be merged in.