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

Add itemProperties to all CatalogGroups #5155

Closed
nf-s opened this issue Jan 20, 2021 · 5 comments
Closed

Add itemProperties to all CatalogGroups #5155

nf-s opened this issue Jan 20, 2021 · 5 comments
Assignees

Comments

@nf-s
Copy link
Contributor

nf-s commented Jan 20, 2021

itemProperties should be available for all CatalogGroups

@wibily
Copy link
Contributor

wibily commented Aug 18, 2021

nick: should be done at groupMixin level unless too hard

@wibily
Copy link
Contributor

wibily commented Aug 18, 2021

this property exists on a few random groups, figure out a way to make this sane

@wibily
Copy link
Contributor

wibily commented Aug 18, 2021

nanda: group can have different items and need to ensure that traits are applicable to all of htem

@tephenavies
Copy link
Member

tephenavies commented Sep 21, 2021

I've read through a lot of groups and my thoughts so far are:

Groups that implement itemProperties (mostly?) do it by setting properties from itemProperties on the underride stratum:

// Replace the stratum inherited from the parent group.
const stratum = CommonStrata.underride;
model.strata.delete(stratum);
model.setTrait(stratum, "name", replaceUnderscores(layer.name));
var uri = new URI(this._catalogGroup.url).segment(layer.id + ""); // Convert layer id to string as segment(0) means sthg different.
model.setTrait(stratum, "url", uri.toString());
if (this._catalogGroup.itemProperties !== undefined) {
Object.keys(this._catalogGroup.itemProperties).map((k: any) =>
model.setTrait(stratum, k, this._catalogGroup.itemProperties![k])
);
}

This works well on a per-group basis, but is hard to port to a Mixin (because each group sets not only the properties from itemProperties, but also additional properties that need to be passed down - note name & url above). Deleting and setting the underride stratum in GroupMixin as well as in the group would result in one of them being destructive to the other.

Thoughts/ideas:

  • Have a new stratum for itemProperties separate to underride for other properties that are passed down
  • I'll have to pass only properties that match the model down, or avoid errors with passing down non-matching properties

This is interesting, potentially useful for creating an itemProperties stratum:

export function createInheritedCkanSharedTraitsStratum(
model: Model<CkanSharedTraits>
): Readonly<StratumFromTraits<CkanSharedTraits>> {
const propertyNames = Object.keys(CkanSharedTraits.traits);
const reduced: any = propertyNames.reduce(
(p, c) => ({
...p,
get [c]() {
return (model as any)[c];
}
}),
{}
);
return observable(reduced);
}
createInheritedCkanSharedTraitsStratum.stratumName =
"ckanItemReferenceInheritedPropertiesStratum";
StratumOrder.addDefinitionStratum(
createInheritedCkanSharedTraitsStratum.stratumName
);

@nf-s
Copy link
Contributor Author

nf-s commented Apr 5, 2022

Closed by #6218

@nf-s nf-s closed this as completed Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants