Fix(ckbox): Improve error handling.

[gorzelinski]

Suggested merge commit message (convention)

Fix(ckbox): Improve error handling. Closes #13062.

---

Additional information

For example – encountered issues, assumptions you had to make, other affected tickets, etc.

[gorzelinski]

The current implementation works, but I'm not sure about it. I considered some alternatives.

1

if ( !isLibraryLoaded ) {
    /**
     * The CKBox library is not loaded.
     * Follow the instructions provided in {@glink features/file-management/ckbox#installation the CKBox feature documentation}
     * to load the library properly.
     *
     * @error ckbox-library-not-loaded
     */
    logWarning( 'ckbox-library-not-loaded' );
    return;
}

if ( !hasConfiguration ) {
    /**
     * The configuration required by the ckbox plugin is missing.
     * Follow the instructions provided in {@glink features/file-management/ckbox#configuration the CKBox feature documentation}
     * to initialize the plugin properly.
     *
     * @error ckbox-config-not-found
     * @param {module:ckbox/ckboxconfig~CKBoxConfig} config Configuration of the ckbox.
     */
    logWarning( 'ckbox-config-not-found', editor );
    return;
}

The code logs warnings correctly, but it's not equivalent to the original code. Tests failed as expected, but I tested it anyway.

---

2

if ( !isLibraryLoaded ) {
    /**
     * The CKBox library is not loaded.
     * Follow the instructions provided in {@glink features/file-management/ckbox#installation the CKBox feature documentation}
     * to load the library properly.
     *
     * @error ckbox-library-not-loaded
     */
    logWarning( 'ckbox-library-not-loaded' );

    if ( !hasConfiguration ) {
        /**
         * The configuration required by the ckbox plugin is missing.
         * Follow the instructions provided in {@glink features/file-management/ckbox#configuration the CKBox feature documentation}
         * to initialize the plugin properly.
         *
         * @error ckbox-config-not-found
         * @param {module:ckbox/ckboxconfig~CKBoxConfig} config Configuration of the ckbox.
         */
        logWarning( 'ckbox-config-not-found', editor );
        return;
    }
}

The code is equivalent to the original. Tests pass. But there is an edge case - nested warning depends on the outer if.

---

3

if ( !isLibraryLoaded ) {
    /**
     * The CKBox library is not loaded.
     * Follow the instructions provided in {@glink features/file-management/ckbox#installation the CKBox feature documentation}
     * to load the library properly.
     *
     * @error ckbox-library-not-loaded
     */
    logWarning( 'ckbox-library-not-loaded' );
}

if ( !hasConfiguration ) {
    /**
     * The configuration required by the ckbox plugin is missing.
     * Follow the instructions provided in {@glink features/file-management/ckbox#configuration the CKBox feature documentation}
     * to initialize the plugin properly.
     *
     * @error ckbox-config-not-found
     * @param {module:ckbox/ckboxconfig~CKBoxConfig} config Configuration of the ckbox.
     */
    logWarning( 'ckbox-config-not-found', editor );
}

if( !hasConfiguration && !isLibraryLoaded) {
    return;
}

Committed implementation works. Warnings are displayed. Tests pass. But it looks a little redundant - I don't know if that's the most elegant solution.

[gorzelinski]

The last commit fixed issues in CKBox unit tests. I also saw warnings in build unit tests. I guess that builds use CKBox. So, I need to handle them as well.

[gorzelinski]

I don't know what to do with failing unit tests, tbh. Added warnings cause CI to crash. It's because the builds per se have ckbox in the list of plugins but no configuration. I tried to mock the config in the unit tests but without results. I'm not sure if that's the right solution in the first place. If I understand correctly, CKBox won't work without config, and our builds don't have one. I could delete the CKBox plugin from the list of plugins - that would solve the issue, but we probably don't want that. I could also add an example config in our builds, but I don't know if we have something like this. Fake config doesn't work. Or I'm missing something. Long story short, currently, I see two solutions:

1. Deleting CKBox from the builds.
2. Adding an example CKBox config to the builds.
3. ?

[gorzelinski]

@pszczesniak found a token used in cloud services tests. In the end, I reused it here. It solved the above issue with the CI. However, our builds still don't have an example config. I don't know if we want to change that.

[arkflpc]

Let's wait for https://github.com/ckeditor/ckeditor5/issues/13062#issuecomment-1759497032 and https://github.com/ckeditor/ckeditor5/issues/13062#issuecomment-1759515299 to be addressed.

[CKEditorBot]

There has been no activity on this PR for the past year. We've marked it as stale and will close it in 30 days. We understand it may still be relevant, so if you're interested in the contribution, leave a comment or reaction under this PR.

[CKEditorBot]

We've closed your PR due to inactivity. While time has passed, the core of your contribution might still be relevant. If you're able, consider reopening a similar PR.