Skip to content

Distinguish external from internal classes #1849

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
davimacedo opened this issue Jul 12, 2021 · 13 comments
Closed

Distinguish external from internal classes #1849

davimacedo opened this issue Jul 12, 2021 · 13 comments
Labels
bounty:$10 Bounty applies for fixing this issue (Parse Bounty Program) type:feature New feature or improvement of existing feature

Comments

@davimacedo
Copy link
Member

New Feature / Enhancement Checklist

Current Limitation

It is currently possible to create a class called User and a class called _User. Both show up in Parse Dashboard with the name User and it causes confusion for some users when using Parse Server for the first time.

In addition to that, it is very confusing when we have to use _User or User throughout the sdks, endpoints, etc.

Feature / Enhancement Description

I believe we should change Parse Server in such a way that User and _User point out to the same thing. So no matter if we send User, or _User, via the api, they will always refer to the _User class we currently have.

Example Use Case

Alternatives / Workarounds

An alternative could be simply disallow the developers to create a User class.

3rd Party References

@mtrezza
Copy link
Member

mtrezza commented Jul 12, 2021

In terms of breaking change, I believe the "alternative" suggestion would not interfere with existing deployments but simply disallow the creation of classes that would conflict with an internal class.

So if a deployment already has User and _User, the alternative approach will not break it. The developer can at their own pace change the name of the collection. If instead we use a redirection of requests from User to _User, it is a breaking change for that deployment.

On the other hand, it is unintuitive to have to prefix internal classes with underscore, while they are displayed without underscore in the dashboard. We could also just display them in their original name in the dashboard.

@davimacedo
Copy link
Member Author

davimacedo commented Jul 12, 2021

Maybe we could disallow the creation of User class and deprecate existing User classes for version 5 and then do the redirect for version 6.

@mtrezza
Copy link
Member

mtrezza commented Jul 14, 2021

How would such a migration work for a live environment to migrate from User to something else before version 6? If this requires special tools beyond Parse Server that we do not provide, I don't think we can put the burden on developers. Maybe we need to add a feature for class renaming (through the dashboard) in a live environment.

Would we still store _User internally, prefixed with underscore? If we do a redirect, that seems unnecessary, but there may he other reasons to keep these classes internal, ie. with underscore.

Just to say that, I guess the simplest way to address the original issue of seeing multiple User classes in dashboard would be to mark the internal classes differently from custom classes. For example just group them at the top of the list, add a horizontal line and then display all custom classes. And we could also display internal classes with their real names in dashboard, including the underscore.

@mtrezza
Copy link
Member

mtrezza commented Jul 24, 2021

Maybe something like this:
image
or without underscore, but that doesn't solve the confusion that in code the system class is _User, but in dashboard it's just User, just in the top group
image

I agree it's not the the most elegant solution, but it may be an easy solution to fix the original issue if migrating existing deployments may be too complicated for developers.

@davimacedo
Copy link
Member Author

We can go in this direction but maybe we should not allow the developers to create User and _User classes anymore. The ones that already have would continue working but we wouldn't allow it anymore for new users.

@mtrezza
Copy link
Member

mtrezza commented Jul 26, 2021

We could do that, or maybe we should keep system class names and custom class names completely separate and not influence / restrict one another, which I think was the original reason why system classes start with an underscore.

Because what happens if tomorrow a new feature introduces a system class _Chat. Will we start to restrict users from creating a custom class Chat?

Any new system class introduction could create significant migration issues for existing deployments. But allowing a _Chat and Chat class at the same time could make it much easier to migrate a custom chat feature to an internal chat feature. And from a development perspective we don't want to have to care what effects the naming of a system class could have on existing deployments.

I think any of these restrictions create the same problem:

  • Prohibit users from creating Class if _Class already exists
  • Redirect requests from Class to _Class
  • Rename system classes _Class to Class as long term goal

So maybe the solution is to make a clear distinction for developers and say:

  • System classes start with underscore
  • System classes don't conflict with or restrict custom class names in any way, but if you create a custom class with the same name (without underscore), it is your responsibility to make sure you don't mix them up
  • New system classes may be introduced in the future with names that are identical to your existing custom class names, but starting with a leading underscore

That means the only measure we would take from this is displaying system classes with either their real name (with underscore) in the dashboard or mark them visually differently in the dashboard, e.g. wrap them in a "System Class" group.

@mtrezza
Copy link
Member

mtrezza commented Oct 5, 2021

@davimacedo If you are fine with it, I would transfer this issue to Parse Dashboard to solve this solely by displaying internal classes differently in the dashboard.

@parse-github-assistant
Copy link

parse-github-assistant bot commented Oct 8, 2021

Thanks for opening this issue!

  • 🎉 We are excited about your ideas for improvement!

@mtrezza mtrezza transferred this issue from parse-community/parse-server Oct 8, 2021
@mtrezza mtrezza added bounty:$10 Bounty applies for fixing this issue (Parse Bounty Program) type:improvement labels Oct 8, 2021
@mtrezza mtrezza changed the title User and _User classes Distinguish external from internal classes Oct 8, 2021
@damianstasik
Copy link
Contributor

damianstasik commented Oct 20, 2021

@mtrezza I tried to experiment with grouping classes visually and for now, it seems like showing the real name would be the simplest solution:

Screen Shot 2021-10-20 at 15 56 04

Would you accept a PR with such change?

@mtrezza
Copy link
Member

mtrezza commented Oct 20, 2021

Sure, would you add a line to separate the internal classes, like in the screenshots in #1849 (comment)? I think it would be a helpful visual guide to faster identify internal classes.

@damianstasik
Copy link
Contributor

damianstasik commented Oct 20, 2021

@mtrezza yes, the PR is ready, and here is how it looks:

https://i.imgur.com/dzfhv7Z.png

The only thing that I am not sure about is the separator API, there are a few ways of doing it, I chose the one that could be reused in other lists if needed.

@mtrezza
Copy link
Member

mtrezza commented Oct 20, 2021

Looks perfect! I'll review.

@mtrezza
Copy link
Member

mtrezza commented Oct 21, 2021

Closing via #1878

@mtrezza mtrezza closed this as completed Oct 21, 2021
@mtrezza mtrezza added type:feature New feature or improvement of existing feature and removed type:improvement labels Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bounty:$10 Bounty applies for fixing this issue (Parse Bounty Program) type:feature New feature or improvement of existing feature
Projects
None yet
Development

No branches or pull requests

3 participants