feat: Custom Plugin Infrastructure with examples provided

[b3lix]

desc: Implementation of custom plugin infrastructure, built along with core application bundle.
Supports PatternFly and also basic HTML components. Provides multiple examples of plugins based on customer use cases. With various customisations of WebUI.

Summary by Sourcery

Implement a custom plugin infrastructure for the FreeIPA WebUI, integrate plugin registration at application startup, and demonstrate the system with example plugins and documentation.

New Features:

• Introduce core plugin system including registry, plugin types, extension slots, and lifecycle hooks
• Integrate plugin registration in the application root and add a Dashboard page with a dashboardContent extension point
• Provide a Hello World example plugin with both HTML/CSS and PatternFly component greetings

Enhancements:

• Update login page branding to use FreeIPA logo and new descriptive text in place of the previous placeholder
• Add plugin developer guide documentation with detailed instructions and best practices

Documentation:

• Add comprehensive plugin developer guide (README) under src/plugins

[sourcery-ai]

Hey @b3lix - I've reviewed your changes - here's some feedback:

• Registering plugins inside a useEffect means they get added after the first render—and since ExtensionSlot doesn’t subscribe to any state, your plugins never actually render; consider registering all plugins before the initial render or hooking the registry into React state/context so extension slots update.
• You’ve exposed an optional reducers field on PluginModule but never wire those into the Redux store at registration—either remove that support or enhance registerPlugin to dynamically inject plugin reducers.

Prompt for AI Agents

Please address the comments from this code review:
## Overall Comments
- Registering plugins inside a useEffect means they get added after the first render—and since ExtensionSlot doesn’t subscribe to any state, your plugins never actually render; consider registering all plugins before the initial render or hooking the registry into React state/context so extension slots update.
- You’ve exposed an optional `reducers` field on PluginModule but never wire those into the Redux store at registration—either remove that support or enhance registerPlugin to dynamically inject plugin reducers.

## Individual Comments

### Comment 1
<location> `src/core/plugins/types.ts:25` </location>
<code_context>
+/**
+ * Type for extension point IDs
+ */
+export type ExtensionPointId = typeof dashboardContent;
+
+/**
</code_context>

<issue_to_address>
ExtensionPointId type is too restrictive for future extension points.

Using 'typeof dashboardContent' restricts ExtensionPointId to a single value, limiting future extensibility. Consider a more flexible type to support additional extension points.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
/**
 * Type for extension point IDs
 */
export type ExtensionPointId = typeof dashboardContent;
=======
/**
 * Type for extension point IDs
 * 
 * Use string for future extensibility.
 */
export type ExtensionPointId = string;
>>>>>>> REPLACE

</suggested_fix>

### Comment 2
<location> `src/core/plugins/PluginRegistry.ts:29` </location>
<code_context>
+    // get the extension point ID string
+    const extensionPointIdString = getExtensionPointIdString(extensionPointId);
+
+    if (!extensions.has(extensionPointIdString)) {
+      extensions.set(extensionPointIdString, []);
+    }
+
+    const currentExtensions = extensions.get(extensionPointIdString)!;
+    currentExtensions.push(extension);
+
+    // sort by priority if provided (higher priority first)
+    currentExtensions.sort((a, b) => (b.priority || 0) - (a.priority || 0));
+  };
+
</code_context>

<issue_to_address>
registerExtension mutates the array returned by Map#get, which could lead to subtle bugs.

Mutating the array in place can cause issues if other parts of the code hold references to it. Use Map#set with a new array to prevent unintended side effects.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
    if (!extensions.has(extensionPointIdString)) {
      extensions.set(extensionPointIdString, []);
    }

    const currentExtensions = extensions.get(extensionPointIdString)!;
    currentExtensions.push(extension);

    // sort by priority if provided (higher priority first)
    currentExtensions.sort((a, b) => (b.priority || 0) - (a.priority || 0));
=======
    const currentExtensions = extensions.get(extensionPointIdString) ?? [];
    const newExtensions = [...currentExtensions, extension];
    // sort by priority if provided (higher priority first)
    newExtensions.sort((a, b) => (b.priority || 0) - (a.priority || 0));
    extensions.set(extensionPointIdString, newExtensions);
>>>>>>> REPLACE

</suggested_fix>

### Comment 3
<location> `src/plugins/hello-world/components/Greeting.tsx:43` </location>
<code_context>
+            boxShadow: "0 4px 6px rgba(0,0,0,0.1)",
+            transition: "transform 0.2s ease",
+          }}
+          onMouseOver={(e) => (e.currentTarget.style.transform = "scale(1.05)")}
+          onMouseOut={(e) => (e.currentTarget.style.transform = "scale(1)")}
+        >
+          ✨ Demo Button ✨
</code_context>

<issue_to_address>
Direct DOM style mutation in event handlers is not idiomatic React.

Use React state or CSS :hover selectors to handle hover effects instead of mutating DOM styles directly.

Suggested implementation:

```typescript
        <button
          className="pf-v5-c-button pf-m-primary greeting-demo-btn"
        >
          ✨ Demo Button ✨
        </button>

```

```typescript
/* Add this style block at the bottom of the file or move to a CSS/SCSS file as appropriate */
<style>
{`
.greeting-demo-btn {
  background: linear-gradient(45deg, #06c, #4a90e2);
  border: none;
  padding: 0.75rem 1.5rem;
  border-radius: 25px;
  box-shadow: 0 4px 6px rgba(0,0,0,0.1);
  transition: transform 0.2s ease;
}
.greeting-demo-btn:hover {
  transform: scale(1.05);
}
`}
</style>

export default Greeting;

```

If you are using CSS modules or a separate stylesheet, move the CSS into the appropriate file and import it. If you are using a CSS-in-JS solution, adapt the style block accordingly.
</issue_to_address>

### Comment 4
<location> `src/core/plugins/PluginRegistry.ts:103` </location>
<code_context>
+    /**
+     * Cleanup all registered plugins
+     */
+    cleanup: (): void => {
+      for (const plugin of plugins.values()) {
+        if (plugin.cleanup) {
+          try {
+            plugin.cleanup();
+          } catch (error) {
+            console.error(`Error cleaning up plugin "${plugin.id}":`, error);
+          }
+        }
+      }
+
+      plugins.clear();
+      extensions.clear();
+    },
+  };
</code_context>

<issue_to_address>
No mechanism to unregister individual plugins or extensions.

Consider implementing methods to unregister individual plugins and their extensions to allow more granular control.
</issue_to_address>

<suggested_fix>
<<<<<<< SEARCH
    /**
     * Cleanup all registered plugins
     */
=======
    /**
     * Unregister a single plugin and its extensions
     */
    unregisterPlugin: (pluginId: string): void => {
      const plugin = plugins.get(pluginId);
      if (plugin) {
        if (plugin.cleanup) {
          try {
            plugin.cleanup();
          } catch (error) {
            console.error(`Error cleaning up plugin "${plugin.id}":`, error);
          }
        }
        plugins.delete(pluginId);

        // Remove all extensions registered by this plugin
        for (const [extensionId, extension] of extensions.entries()) {
          if (extension.pluginId === pluginId) {
            extensions.delete(extensionId);
          }
        }
      }
    },

    /**
     * Unregister a single extension by its ID
     */
    unregisterExtension: (extensionId: string): void => {
      extensions.delete(extensionId);
    },

    /**
     * Cleanup all registered plugins
     */
>>>>>>> REPLACE

</suggested_fix>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[duzda]

Hi @b3lix,

since we're hopefully releasing as well, just a note.

[carma12]

@b3lix - Friendly reminder to remove the line mentioned here and push the changes to rerun the tests :)

[duzda]

Hi, you will have to rebase,

git rebase -i HEAD~3, there go and drop 9539a039a537a23ac9f44cbd64be90cdd27032601

Then you can run
git rebase origin/main

And adapt the PF6 changes.

Please make sure to not create merge commits, they just complicate stuff 🙂

[github-actions]

This PR has not received any attention in 60 days.

[b3lix]

Hi @duzda
rebased and patternfly updated to version 6.
Please re-review, nothing is visible on UI now, plugin just as an examples in code.
After implementation of Dynamic Custom plugin infrastructure, not needed parts of this will be removed.

[duzda]

Hi, my fundamental issue with this PR is, that this should be solved outside of this codebase, apart from the Dashboard I can't think of how to utilize the plugins for the dynamic version. I'd just go back to the drawing board. I simply do not understand how one can utilize plugins implemented this way for their own benefit.

This comment is very important, the rest is just code suggestions, that do not solve the fundamental issue in any way.

I'd stay away from creating core/plugins folder, it makes more sense to utilize the plugins folder for that and simply have core there.

[duzda]

It's not clear that this is props.

[duzda]

camelCase for .ts files

[duzda]

This is never used anywhere?

[duzda]

This is nowhere used.

[duzda]

PluginsDashboard.tsx

[duzda]

This is not very PF React friendly.

[duzda]

We typically stay away from index.ts, the reasoning, adding abstractness, hard to follow, slower, there's multiple articles about this topic, for the test it makes sense.

[duzda]

We actually mostly have CamelCase for components.