silverstripe / silverstripe-taxonomy

Provides the capability to add and edit simple taxonomies within Silverstripe
BSD 3-Clause "New" or "Revised" License
9 stars 24 forks source link

Taxonomy directory - problems to be solved #55

Open robbieaverill opened 6 years ago

robbieaverill commented 6 years ago

I've just had a look at the TaxonomyDirectorController in the context of CWP 2.0. Here are some issues I've noticed:

I actually think that this class shouldn't be in this module - BasePage is specific to CWP, and this logic should live in the cwp/cwp module or something similar.

I think we should deprecate the class and move it back to a documentation only example. If it should live in CWP then we can add it to CWP later on. Thoughts?

NightJar commented 6 years ago

The PR is https://github.com/silverstripe/silverstripe-taxonomy/pull/45#discussion_r160744058 with the comment thread in question (which is aimed to supercede #40 by the PR description).

I feel like dot notation can solve the issue of the phantom BasePage, and remove the innerJoin custom query logic. This is particularly helpful in SS4 with different/adaptive/settable table names. The dot notation reference can be built by a relation lookup, which can be defined via injector service definitions.

Caveat that I've not thought too deeply about this yet. Just initial thoughts.