[mathesar-developers] Research: Publicly shareable links

[Pavish Kumar Ramani Gopal]

Team,

I've written up a project to research Publicly shareable links and Embedding tables & explorations on other sites. I got approval to look into these features from Kriti before I went on vacation.

The outcome of this project would be to produce a high level Product spec and get it approved by the team, so that all of us are on the same page with respect to the features. Once we have the product spec, I'll write up a new project to track design and implementation.

Please feel free to provide feedback w.r.t to the project description and thoughts & ideas that might help with the research.

Thanks!

[Pavish Kumar Ramani Gopal]

Team,

The Product spec for Publicly shareable links: https://wiki.mathesar.org/en/product/specs/publicly-shareable-links is ready for review.

Please review it in the following aspects:

- Product aspect + Final approval: @Kriti Godey 

- Design aspect: @Ghislaine Guerin 

- Backend approach: @Brent Moran @Mukesh Murali  (Brent for the DB schema and overall approach, Mukesh to verify the authentication related approaches)

- Frontend approach: Does not have enough content to require review at the moment (Sean & Rajat, please feel free to look at it). I'll request a review during the implementation phase.

Please let me know if you have any questions or thoughts/suggestions.

Thanks! 

[Pavish Kumar Ramani Gopal]

A few notes I missed to add in my last email:

The suggested approaches and UX flow are pretty high level. This is meant to be a starting point. If this looks good enough, we could discuss detailed implementation logic in PR reviews or subsequent chats during the implementation phase.

I intend to work on both the backend and the frontend for this project. I'd like support with respect to PR reviews and discussions from the backend team.

For the design, I think it's straightforward enough to build the frontend and get the PRs reviewed from Ghislaine.

Let me know if you have concerns. Thanks!

[Pavish Kumar Ramani Gopal]

Brent and I had a quick call over this, and we discussed the following:

• It's not a great idea to have a polymorphic relationship and it'd be better to avoid having triggers in place of fkeys.
• For the backend, it'd be cleaner if we had both the object information and the token in the requests.
• This would remove the need for having a separate `public_links` model.
• It would make it easier to implement this for other objects in the future.
• For the frontend, it'd be cleaner to have the public urls scoped within a different route.

We found the following approach more reasonable, which satisfies the concerns we had:

• Instead of a separate model for links, we'd have a field in all the individual models for the public link.
• Eg., The table & query models would contain a field `public_link_hash`: `f2eea1b0-591f-4414-89ae-87d1688bf1d6`.
• The generated link will contain both the object information and the hash.
  
• Eg., `/db/mathesar_tables/1/tables/3/?access_key=f2eea1b0-591f-4414-89ae-87d1688bf1d6`.
• The frontend routing for public paths would be namespaced.
  
• Eg., The table public path would look like: `https://domain.com/public/db/mathesar_tables/1/tables/3/`.
• It would have the same path as the table, except notice the prefix `public` above.
• Overall, the generated link would look like this: `https://domain.com/public/db/mathesar_tables/1/tables/3/?access_key=f2eea1b0-591f-4414-89ae-87d1688bf1d6`
• This eliminates the need to have a dedicated `public_links` model and having triggers to clean up stale entries, making it cleaner on the backend.
• It also resolves the frontend concern of having namespaced routes for public links.

Note, the above discussion does not affect the API urls since the original design already mentions that the hash would be provided to the same endpoints.

@Brent Moran Please let me know if I had missed anything here.

For the other reviewers, I have not yet the product spec with Brent's reviews. I am in full agreement with the conclusion we came up with, I want to wait till everyone is done reviewing before I update the spec.

[Brent Moran]

I think this covers it as per my understanding. I do think it's a good idea for @Mukesh Murali to weigh in on the auth part of things before getting too far along, though.

[Sean Colsen]

I'll add some thoughts:

• Nice doc, Pavish! Very well-structured and clearly written. I’m happy to see this sort of process in building a new feature.

• The spec says:

  Tables will contain the menu bar with options to filter, sort, and group.

  What about explorations? Can we show the same filter/sort/group UI for those too, so that viewers can manipulate the results of an exploration?

• Regarding the data model…

  Here is a nice article which I’ve referenced in the past, and will reference here for the sake of discussion.

  The article classifies Pavish’s original suggestion as “polymorphic joins” and mentions the downsides that it sounds like Brent also found concerning. I agree we should avoid it.

  Brent’s counter proposal is similar to the “reverse belongs-to” approach, as classified by that article. However, in Brent’s proposal, there would not be a “public_links” model at all.

  Brent’s proposal would seem fine to me if we had no intention of ever associating any extra settings with a public link. But our competitors do associate extra data with sharable links. For example Baserow lets you add a password to the link and Google Sheets lets you choose the level of access to grant to visitors. In the “Scheduled for later iterations” section of the spec, I can see that we already have our eye on building similar features, so I would suggest establishing a data model at the outset which is more amenable to extension.

  The article concludes by recommending “exclusive belongs-to”, which I think is worth considering here. We would have a public_links model, like Pavish recommended, but instead of having entity and entity_id, it would have table_id and exploration_id, along with an XOR-style CHECK constraint for integrity purposes. What do you think about that, Brent and Pavish?
  

​

[Kriti Godey]

Nice work Pavish! Please consider the project approved.

Regarding the product spec:

• The description of how the feature should work sounds good. I think it makes sense to do the simplest possible version first and add more features later.
  
• I agree with Sean that we probably want to associate metadata with links in the long run and it's better to plan for that now.
  
• An alternate data model could be to have different models for different types of public links (e.g. TablePublicLink with an FK to a table, ExplorationPublicLink with an FK to an exploration). This will also allow us to have multiple links per object in the future (e.g. for different filtered and grouped views, or with different expirations, or different passwords, or different permissions, or whatever).
• To make this efficient to implement in the backend and centralize common logic, we could have an abstract PublicLink model that the other models inherit from.
• This is a minor detail, but model names should be singular and use CamelCase.
• We don't need triggers for any of this. Django has a bunch of options for what to do when models are deleted.
  

[Pavish Kumar Ramani Gopal]

Thanks Kriti and Sean!

I agree we would have to factor in metadata. I'll do some reading up on the links you've sent along with more of the Django docs, have some chats with Brent and Mukesh and come back with an updated document. I had a call with Mukesh yesterday and he had a concern about the same and suggested I check out the generic foreign key feature provided by Django.

On a side note, Mukesh approved the high level implementation approach with respect to authentication and authorization behaviour.

To answer Sean's question:

> What about explorations? Can we show the same filter/sort/group UI for those too, so that viewers can manipulate the results of an exploration?

This is out of scope for the Publicly shareable links feature.

We currently do not have this feature implemented on the frontend in the Explorations page. We do have the necessary APIs implemented on the backend.

I would definitely like to work on it and it is on our roadmap, and we can pick it up as a separate project.

[Ghislaine Guerin]

Thank you for presenting the design. It is nicely structured, and though high-level, it covers most of the essential points. I just have a few notes I wanted to share with you.

Distinction Between User Types
Given that the shareable links are also exposing Mathesar's product to a wider audience, I believe it is crucial to consider the two distinct types of users separately: the user creating the link, and the user consuming the link or embedded content.

User Experience and Accessibility
It is important to focus on the experience of the users consuming the embedded content. Specifically, consideration needs to be given to the context in which the tables are being embedded. We should also take into account how the embedded content will be displayed through different devices and ensure that it is accessible to all users.

Sharing Modal and Previews
Regarding the modal, I think the design is logical. Users should ideally have the option to open the public link in a new window for them to see what it would look like to others. Additionally, a preview of the embedded table or exploration could be highly beneficial to users, though I understand that this feature could be added in subsequent iterations.

Comparison to Google Sheets
As a side note, I noticed that in the document, the section on Google Sheets mainly mentions options for providing access to the table. However, it's important to point out that Google Sheets also has a dedicated "publish" setting that aligns more closely with what is being described here, which focuses on providing access to the actual table as a reader. This could serve as a good reference.

In summary, this design appears to be on the right track :) Thank you for working on it

[Mukesh Murali]

> As a side note, I noticed that in the document, the section on Google Sheets mainly mentions options for providing access to the table. However, it's important to point out that Google Sheets also has a dedicated "publish" setting that aligns more closely with what is being described here, which focuses on providing access to the actual table as a reader. This could serve as a good reference.

When I had a call with Pavish I brought up the topic of associating access control(anyone with this link can edit this table), Pavish corrected me by saying the current feature is just for publishing a Table and not about associating with metadata or having access control(due to the time constraint, I will comment on this later). This aligns closely with what Ghislaine is pointing out in the comment. It would be best for @Pavish Kumar Ramani Gopal to clear out the confusion when updating the spec.

> . I had a call with Mukesh yesterday and he had a concern about the same and suggested I check out the generic foreign key feature provided by Django.

I didn't want to go too deep into this topic as we didn't have a need for this feature. But for posterity, Django [Generic Foreign key](https://docs.djangoproject.com/en/4.2/ref/contrib/contenttypes/#django.contrib.contenttypes.fields.GenericForeignKey) is not something I would rely on as it is not based on database relations which could lead to a lot of problems(portability issues, transaction failures, pitfalls/bugs in Django). I would instead look into [Django polymorphic](https://django-polymorphic.readthedocs.io/en/stable/quickstart.html) or [Django model-utils Inheritance Manager](https://django-model-utils.readthedocs.io/en/latest/managers.html#inheritancemanager) as they are based on actual database relations


The below comments are more of a reference on the polymorphism implementation details so feel free to skip them if we don't plan on associating metadata with the links

> Brent’s counter proposal is similar to the “reverse belongs-to” approach, as classified by that article. However, in Brent’s proposal, there would not be a “public_links” model at all.

Comparing the proposals based on the article sent by Sean.
- Pavish proposal is the same as Django [Generic Foreign key](https://docs.djangoproject.com/en/4.2/ref/contrib/contenttypes/#django.contrib.contenttypes.fields.GenericForeignKey) except all the chore is taken care by Django so that it mimics a `Foreign key` field.
- Brent's proposal does not have anything to do with polymorphic relations. But you are right in comparing it to *Reverse belongs to* and I actually don't see what's the purpose of the *Reverse belongs to* with respect to polymorphism because you won't be able to add additional fields for the inherited entities without affecting other entities which are not possible in this case.

- Kriti's proposal is the *Join Table Per Relation type* or [Django abstract inheritance](https://docs.djangoproject.com/en/4.2/topics/db/models/#abstract-base-classes)

- [Django polymorphic](https://django-polymorphic.readthedocs.io/en/stable/quickstart.html) or [Django model-utils Inheritance Manager](https://django-model-utils.readthedocs.io/en/latest/managers.html#inheritancemanager) uses the *Exclusive Belongs To* or [Django concrete inheritance](https://docs.djangoproject.com/en/4.2/topics/db/models/#multi-table-inheritance)approach mentioned in the article except it does not use a sparse table instead uses Django model dependencies to find out the related Tables to perform an `INNER JOIN` during runtime. The libraries makes it much easier to query and join the inherited models compared to [Django concrete inheritance]


We could implement *Exclusive belongs to* without a Library but a library abstracts all the complexities away. Kriti's proposal is the simplest as it does not introduce additional libraries. But there are a few drawbacks to it.

- We won't be able to query all the links together easily as we need to know which type of query it is. For example `/public/<hash>` won't be possible, instead, it has to be `/public/table_links/<hash>`. We could query across all types of link objects but at that point, I would prefer to go with *Django concrete Inheritance*
- Functionalities like analytics or anything common to abstract links will have to be duplicated for all the links.

[Pavish Kumar Ramani Gopal]

Thanks Ghislaine and Mukesh!

The idea is to start with publicly sharing read-only tables & explorations and later extend it to have options like allowing editing and restricting with password. However, these links relate to public urls and not in-app access control related, which do not require links. We currently have access control for databases and schemas, and later on, it would extend to tables and explorations. That is a separate feature. I understand how this might be confusing when reading the spec, I will update it to make this more clear.

Answering Ghislaine, 

 > it's important to point out that Google Sheets also has a dedicated "publish" setting that aligns more closely with what is being described here, which focuses on providing access to the actual table as a reader

I agree. In my mind, this was implicit and I was focused on documenting things that were different in each tool, and missed out documenting this, which was an oversight on my part. I will update the spec.

I will also update the spec with all the points you've mentioned in the mail.

Answering Mukesh,

Thanks for the run down of the options we have. I will consider them and get back with an approach. If I have questions, I will reach out to you directly.

[Kriti Godey]

Thanks Pavish! To confirm, there's no action needed from any of us until you share an updated spec, right?

[Pavish Kumar Ramani Gopal]

Yes, Kriti, that's correct. I'll ask for another review once the spec is updated.

[Pavish Kumar Ramani Gopal]

Team,

I've updated the spec with the backend approaches and my recommendation. Please review it.

@Brent Moran, @Kriti Godey, @Sean Colsen, and @Mukesh Murali Since you've all provided your opinions, it'd be great if you took a quick look at it and let me know what you think about my recommendation. TLDR: It's the same as Kriti's suggestion mentioned in the previous mail.

Ghislaine, I've addressed your concerns in the spec. I don't think it needs review, but please feel free to take a look.

Thanks!

[Brent Moran]

I agree that the 'one dedicated table per entity' approach makes most
sense of the options given, and I can't think of any glaring omissions
(taking the constraints for granted). While the repetition looks
irritating in the DB diagram, the actual Django implementation will be
quite standard.

On Mon, Jul 3, 2023 at 9:57 PM Pavish Kumar Ramani Gopal

[Sean Colsen]

Looks great! The "Approaches considered" section is a nice summary. I really appreciate when specs include stuff like this! And wow, those ER diagrams! Woo! Out of curiosity, what did you use to create them?

[Kriti Godey]

Looks good to me too.

My only (small) concern is that maybe we shouldn’t name the URL public_links (we might want to use the same feature for password protected links, or whatnot, which aren’t “public” in the same way, and could lead to confusing mental models in the future). I’d consider something more generic like shares instead of public_links.

[Pavish Kumar Ramani Gopal]

Thanks everyone!

I've created an Implementation project for Shares.

I've also updated the spec with some additional details and API endpoint specifications. It doesn't require a review, but please feel free to take a look.

I think we can discuss anything further in PR reviews since they are likely to be implementation related, and I'd like to consider the product spec to be complete. Let me know if any of you have concerns or questions.

> While the repetition looks irritating in the DB diagram, the actual Django implementation will be quite standard

I'm having a hard time dealing with an internal resistance against this DB schema, but it seems to be the most reasonable approach when looking at the point of view of Django models. It's gonna take me a while to get used to the Django world.

> ER diagrams ... Out of curiosity, what did you use to create them?

I used https://www.quickdatabasediagrams.com/ to create them. It's been my goto tool for years now.

> I’d consider something more generic like shares instead of public_links

I agree and I'd like to move ahead with `Shares`.

I had a concern initially because we seemed to use the word `share` in other places in the app and that might affect readability, but having a concise term is better and I can't think of anything well suited other than 'shares'. If any of you have suggestions, let me know.

[Kriti Godey]

Thanks Pavish!