All work
- AbstractClassPackage must support activated_classcollectionsCELDEV-516Resolved issue: CELDEV-516Marc Sladek
- ImageUrl should always return unencoded URLsCELDEV-486Resolved issue: CELDEV-486Edoardo Beutler
- Accordeon JS failes when active content is emptyCELDEV-485Resolved issue: CELDEV-485Fabian Pichler
- TabEditor / saveWithValidation no failed message on DocumentSaveExceptionCELDEV-484
- Newsletter plain text conversionCELDEV-483Resolved issue: CELDEV-483Gal Cohen
- DocumentCacheStore: API breakage leads to cache poisoningCELDEV-462Resolved issue: CELDEV-462Fabian Pichler
AbstractClassPackage must support activated_classcollections
Activity
Fabian Pichler 18 July 2017 at 19:47(edited)
approved.
I have seen, that you fixed PageTypeClassPackage in the same PR.
Yet, please add issues to the projects needing a fix for the other packages to change them as soon as the new celements-core is released and adopted.
Marc Sladek 17 July 2017 at 09:34
@Fabian Pichler I like the suggested improvements see PR
Fabian Pichler 16 July 2017 at 13:43
I would prefer to distinguish between Packages needing a legacy fallback and new Packages never been available as ClassCollection. This could be achieved by providing two different abstract classes: AbstractClassPackage and AbstractLegacyClassPackage only the second implements isActivated with a fallback lookup and logs a warning that the fallback was needed and might be removed in future versions.
wdyt?
Marc Sladek 10 July 2017 at 15:22
Commited work for suggested solution: https://github.com/celements/celements-core/commit/8218502016ddf45c9a8009ce86c9d845879e18da
Marc Sladek 10 July 2017 at 12:41(edited)
Yes, I see the problem. Another idea: why not add a legacy fallback to ClassPackage.isActivated() which, if the default implementation would return false, checks e.g.:
@Override
public boolean isActivated() {
[...]
if (!activated && getLegacyName().isPresent()) {
String preferences = "," + context.getXWikiContext().getWiki().getXWikiPreference(
"activated_classcollections", context.getXWikiContext()) + ",";
activated = preferences.contains("," + getLegacyName().get() + ",");
}
return activated;
}
protected Optional<String> getLegacyName() {
return Optional.absent();
}
The legacy config name can be overwritten in the affected ClassPackages
I think we have a big missconception. We use the AbstractClassPackage as a "dropin replacement" for the to replace the AbstractClassCollection. This is actually NOT working as intended because if you have added a classCollection name to your XWikiPreferences field activated_classcollections then this ensured that the class of your selected extension are getting generated. YET if the developement of your extension is changing to use the new AbstractClassPackage the classes won't be generated and updated anymore.
We need to get back and check for all replacments of usages of the AbstractClassCollections with the AbstractClassPackage and need to write for each replacement a Migration checking on the XWikiPreferences and converting the activated_classcollections field to the new celements.classdefinition.active