Skip to content
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

Refactor ViewableData (aka ModelData) in CMS 6 #11385

Open
GuySartorelli opened this issue Sep 17, 2024 · 0 comments
Open

Refactor ViewableData (aka ModelData) in CMS 6 #11385

GuySartorelli opened this issue Sep 17, 2024 · 0 comments

Comments

@GuySartorelli
Copy link
Member

GuySartorelli commented Sep 17, 2024

Note

In #11322 we're renaming ViewableData to ModelData. I'll refer to it as ModelData for the rest of this issue.

Current state

ModelData is the superclass of way too many classes in Silverstripe CMS. For example, these are some of the classes which are not models but which are subclasses of ModelData

  • RequestHandler (and as a result: FormField and Controller)
  • BulkLoader (e.g. for importing CSV data)
  • SilverstripeNavigator and SilverStripeNavigatorItem

These classes need at most some small subset of the functionality provided by the current ModelData class.

What's more, DataObject currently holds way more logic than purely what is needed for the ORM functionality of that class. The searchable and summary fields logic for example is not limited to data stored in the database.

Proposal

  1. Split up ModelData based on the functionality that its existing subclasses actually need. Whoever implements this will need to determine how to best split it up, and whether its into classes in a hierarchy or traits or extensions or some combination there-of.
    1. new MagicAccess class (or trait? Probably class) with getField(), setField(), __get(), __set(), the failover logic, etc. Anything related to accessing or setting data generically. The name is meant to mimic ArrayAccess
    2. ModelData still exists, and is a subclass of the other classes (or uses the traits, or implements the interfaces - however it ends up being implemented)
    3. DataObject is still a subclass of ModelData - every other subclass of ModelData is reviewed to check what functionality it needs and its inheritance is updated accordingly.
  2. Move logic out of DataObject that isn't needed for ORM interactions.
    1. Could be moved into traits, further up the class inheritance chain, into extensions, or even just make an interface but keep the logic itself in DataObject if that's most appropriate.
    2. We could have a Searchable interface for searchable fields, search context, etc and a Summarisable interface for summary fields, for example.

Then DataObject only has what it needs for ORM interaction (db fields, relations, storing and fetching from the DB, interacting with DataObjectSchema, etc) and everything else can be used in other models and functionality checked for using type hints.

That makes it way easier to use non-DataObject in places that currently "need" DataObject to function (e.g. when using a 3rd party service or with data that is otherwise not directly pulled from the DB)

It also means things like RequestHandler don't end up with methods they just don't need (like obj(), xml_val() etc which used to be needed for the template layer but won't be after #11322 is complete)

Possibly related issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant