[CONTTAGS-27] Define what content is taggable Created: 11/May/17 Updated: 18/Aug/17 Resolved: 18/Aug/17 |
|
| Status: | Closed |
| Project: | Content Tags |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 1.0 |
| Type: | Task | Priority: | Neutral |
| Reporter: | Antti Hietala | Assignee: | Roman Kovařík |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||
| Template: |
|
||||||||||||
| Acceptance criteria: |
Empty
|
||||||||||||
| Task DoR: |
Empty
|
||||||||||||
| Date of First Response: | |||||||||||||
| Epic Link: | Tagging | ||||||||||||
| Sprint: | Kromeriz 109 | ||||||||||||
| Story Points: | 5 | ||||||||||||
| Description |
|
User story:
Tasks:
See: Tagging content: Technical prerequisites, recommendations |
| Comments |
| Comment by Jan Haderka [ 22/May/17 ] |
Please be aware that this should be used only for concrete instances of the content of given type. Doesn't apply for whole type (otherwise registration would need to happen on the node type and applied on all existing nodes of said type and would affect also of all subtypes => chain reaction, lot of mess for no benefit). The purpose of the mixin is to declare property/ies that node instance can be assumed to contain. In this case links to the tags with which content was tagged. It also allows simple search for all tagged content within given workspace (select * from mgnl:taggable). Use of mixin to declare what given instance of node contains vs. defining what types of content can be tagged are two different things and should be treated as such.
mgeljic Why? References are evil. What is the benefit of using weak reference? |
| Comment by Milan Divilek [ 28/May/17 ] |
|
WEAKREFERENCE
see for more details Simple STRING property should be enough for our use-case. If we want to store tags only in one workspace then “mgnl:tags” property should contain just uuids. If there is plan to store tags in more workspaces then we can use pattern like “workspace:uuid”. What content is taggable
"mgnl:taggable" mixin would require to re-register the nodetypes when Magnolia is operating. It's ok to re-register node type's when Magnolia is in "instalation" status, but I don't like idea to re-register it when Magnolia is operating. |
| Comment by Antti Hietala [ 29/May/17 ] |
One workspace to store tags should be enough.
The goal of the user story is to make a developer's life easier. It should be a simple task to enable tagging in an app – ideally a single configuration point rather than many. If adding a mixin on the node type is problematic try to think of other ways to fulfil the story. Could the single configuration point be in the app descriptor? "By setting this property I make all content in the Tours app taggable." This might be too broad as it would make folders taggable. Or could it be a switch in the content connector where we define the node types the connector operates on?
subapps:
browser:
contentConnector:
nodeTypes:
- name: mgnl:content
icon: icon-node-content
taggable: true
|
| Comment by Milan Divilek [ 30/May/17 ] |
|
Discussed on architecture meeting: Decided to park the ticket until Magnolia content types effort is ready and proceed with other tickets DEV-515. see more https://wiki.magnolia-cms.com/display/ARCHI/2017-05-30+Architecture+meeting+notes |
| Comment by Mikaël Geljić [ 30/May/17 ] |
|
had,
—agree to disagree? The mixin has little value if it's tied up 1-to-1 to the presence of the mgnl:tags property. We're just informing what the model is, and avoid polluting content. Without the burden of keeping the mixin in sync too.
That. Tags must be added by their identifer; sparing us few lousy conversions and sanity checks.
assertThrows(javax.jcr.ValueFormatException.class, () -> {
content.setProperty("mgnl:tags", new String[] { "invalid-multi-value" });
// javax.jcr.ValueFormatException: java.lang.IllegalArgumentException: invalid-multi-value
});
Re: javax.jcr.Property#getNode, how is a String any better there? Different exception, same-same; besides with multi-value you cannot call #getNode or #getString straight anyway. Weak reference is precisely for UUIDs without any referential integrity at all (neither workspace nor repo).
No thanks, or not before the other initiative about referencing/linking content.
Not true.
What if we stop thinking about apps? We have way more than enough "loose modeling" scattered through e.g. actions, node-wrappers. The more we do this, the less open we are towards headless mgmt, eventually.
Me neither, at least not until we have a solid backbone proving Magnolia can do it safely for you.
ahietala, this does not need depending on the content-types effort. Just the person defining it is different (dev vs. general content modeler). |
| Comment by Jan Haderka [ 05/Jun/17 ] |
Nope. Spec is rather explicit about this "Every node has one declared primary node type and zero or more mixin node types. Primary node types are typically used to defined the core characteristics of a node, while mixin node types are used to add additional characteristics often related to specific repository functions or to metadata."
You are missing the point, week references can be applied only within the workspace not across the workspaces so they do not work for what we have in mind here. On the contrary, if you use the week reference, you might be obtaining WRONG node, by calling getNode() method on the weak-ref property if node w/ same UUID accidentally exists in same workspace. It's not that weak references are "workspace free", rather repository makes assumption that they are within same workspace. This kind of opens whole new area for misunderstandings, mistakes and misconceptions between those who would be required to use our code (why is there some method from JCR spec that is suddenly returning invalid code and which use must be avoided?)
IMO this is the crux of the matter. We have reached the point where what we understand under content type goes beyond how Magnolia uses underlying repo and even beyond of what NodeType is in terms of repository. We have overloaded the concept to the point where repo NodeType is just subset of functionality provided by repository and use of the type within UI is another subset. As such we should be declaring our own Content/NodeType that encompasses all current and foreseen uses and not limit ourself to capabilities of the repository as we will eventually need to apply the same to other data sources than just JCR ones. Use of mixin for marking tagged nodes is implementation detail of tagging within JCR, use of flag/mixin on *Magnolia Content Type* to declare type as taggable (and thus allowing us automatically allow use of mixin in repo or deployment of extra functionality/actions in apps or automatic resolving of such tags when producing JSON from REST endpoints is not JCR repo specific and should NOT be tied to JCR Node Type). |