Update module name to DNN Community Forums

[johnhenley]

Is your feature request related to a problem?

The forums module was inherited as part of the acquisition of Active Modules by DNN and then open-sourced and has been through a few iterations.

There are several visible and internal references to Active Forums, including the installation process, logos, etc.

This also includes the module name itself within the instance, and the activeforums_ prefix on all database objects.

Describe the solution you'd like

This will be a multi-part endeavor and needs to be well-planned and executed and thoroughly tested. It will likely span multiple releases and each piece (database vs naming vs graphics) can be done independently.

[WillStrohl]

Specific things I can think of to include off of the top of my head, include:

• Package name (manifest & database)
• Friendly names (package & module)
• Project/DLL properties
• Solution file
• Copyright/product name in the UI (if this still exists)
• Resource files (localized text)

Things that will probably not be rebranded include:

• Namespaces
• HTML
• Templates
• CSS

Did I miss anything?

[johnhenley]

There are some rows in the scheduler tables that have the friendly name in them: @FriendlyName = 'Active Forums Mail Queue' There are some literals sprinkled around of course, but for the most part, the code itself is pretty clean since I put a constant in Globals.cs to use for the Module Name. public const string ModuleName = "Active Forums";

I'm assuming you don't want to tackle the folder under DesktopModules yet?

[WillStrohl]

Nice catch on the scheduled task!

Good point about the folder... We're not putting the templates in the /DesktopModules/ folder, right? As long as that's true, we technically could do that. There's risk, but it's a reasonable change to consider. 🤔

[Timo-Breumelhof]

Ok, I have no idea how much extra work that would be but wouldn't it be better to do a complete convert including the desktop modules folder? To avoid having to do that later and test again etc.?

[johnhenley]

We're not putting the templates in the /DesktopModules/ folder, right?

Actually they are still in the /DesktopModules/ folder. /DesktopModules/ActiveForums/theme//templates/ for instance.

In my original ginormous pull request, I had moved them under the portal root but that PR just had too much in it so I broke it into the 3 PRs and just refactored the previous folder changes to (mostly) rely on a constant for the module root and variables rooted off of that. So theoretically changing the location is easier. However there were some cases (in user control ascx files) that I had to restore the hardcoded folders :) and punt.

[johnhenley]

Ok, I have no idea how much extra work that would be but wouldn't it be better to do a complete convert including the desktop modules folder? To avoid having to do that later and test again etc.?

My experience with this module thus far is that "big bang" changes aren't feasible. I tried that with the first big pull request for moving templates from the database and had to carve that into smaller (yet still huge) chunks.

None of us sit in front of this project on a daily basis and testing is difficult and probably not widespread. It's much more effective to do small incremental changes and get them out the door.

My preference for the rebrand is to do it in this order (with group 1&2 combined perhaps).

Group 1:

• logo/icon and initial install graphic
• user-facing literals/string resources
• admin-facing literals/string resources

Group 2:

• package/manifest
• internal module name
• scheduler entries (those are changing in future PRs -- :) )

Group 3:

• desktop module folder

Group 4:

• Namespaces

Group 5:

• Database objects

[Timo-Breumelhof]

ok :-)

[WillStrohl]

Great points, John! I like your proposed approach. However, I'm not sure we need to worry about Groups 4 & 5. There isn't much to gain from those changes. Do you agree?

[WillStrohl]

@johnhenley I have a PR coming that does a lot of this. However, we'll need to perform the DNN object naming in two different releases to be sure it's a clean break, I think. Correct me if I'm wrong, but here's what I'm thinking...

The upgrade process is entirely dependent on the existing Package Name and Module Name.

If we were to only rename the Package Name and Module Name in the DNN manifest and upgrade, everything appears to work okay, but an orphaned record is created in the [DesktopModules] and [ModuleDefintions] table that is effectively doing nothing. It's just there. This means that we'd have to maintain the Package Name of Active Modules, as it would need to make the package name in the manifest file as well.

Instead, I'm preparing a two-step upgrade process. I'll need your feedback to be sure I'm thinking this through correctly.

1. Change the Package Name, Module Name, and Friendly Name in 8.0 via IUpgradeable. The manifest will not have any changes, because it needs to correctly trigger the upgrade.
2. Immediately following the 8.0 release, prepare and merge a PR that updates the Package Name, Module Name, and Friendly Name in the DNN manifest, so all future upgrades have these values.

What do you think? 🤔

[WillStrohl]

@johnhenley In addition to my previous comment...

I didn't do the work for the renaming of the scheduler jobs. I gathered from a previous comment that you're planning to do that already.

I've rearranged the groupings and tasks a bit below, based on what I think we are and should be doing. I've marked what I think this PR will have had us completed.

Release 8.0

Group 1:

• [ ] Module branding logo/icon (waiting on Timo)
• [ ] First-time install page load graphic/logo (waiting on Timo)
• [x] user-facing literals/string resources
• [x] admin-facing literals/string resources

Group 2:

• [x] Package
• [x] Internal Module Name
• [ ] Scheduler Entries (waiting on John)

Subsequent Release(s)

Group 3:

• [ ] Manifest (requires 8.0 to be released first)

May Not Be Worth Doing

Group 4:

• [ ] Rename DesktopModules folder
• [ ] Database objects

Group 5:

• [ ] Namespaces
• [ ] DLL(s)

[WillStrohl]

@johnhenley In addition to my previous two comments... 🤦🏽‍♂️

Here's the thought process behind the updates in the PR (yes, I'm referencing the first comment now). 😁

Upgrade Logic Workflow

This update intends that when people upgrade to version 8.0, the upgrade will update the database with the names that we intend to have. These values will have an immediate effect on renaming the module in the UI.

However, the DNN manifest will still have the legacy names in it during this upgrade. This is necessary to trigger the upgrade logic for the original module, before it's renamed.

After version 8.0 is released, all future iterations of the DNN manifest should be using the new names.

Potential Use Case Issue

I was just thinking... What would happen if we have several versions of 8.0.x and maybe 8.1.x, and more?

If someone is still running 7-something and doesn't try to upgrade until 8.1+, will the upgrade fail?

Will they first be required to upgrade to 8.0, then upgrade to any version past that? If yes, is there a way to enforce that? I don't think there is. 🤔

[johnhenley]

Hmm. I posted a comment but deleted it because I realized I really need to dig into the installer code and see what options we might have. The skip-upgrade scenario definitely introduces a challenge:)

[WillStrohl]

Okay. I look forward to hearing about what you find. :)

[johnhenley]

Based on my analysis, these are the different install/upgrade scenarios we need to expect and address, including paying particular attention to the 8.1+ upgrades. The most problematic scenario is a 'skip upgrade', for example, from 07.00.07 to 08.01.00. Since we are handling the rebranding in the 08.00.00, there are a number of tasks that need to be properly handled to ensure a smooth upgrade process. We don't want to require any additional steps on behalf of the user/installer. We want to avoid needing a special "upgrade" package or a multi-step upgrade scenario.

Currently installed	Installing	Expected result	Notes
none	CF 8.0	New install of CF 8.0	detect as new install and install CF 8.0
AF 7.0.x or lower	CF 8.0	AF upgraded to CF 8.0	Detect AF and upgrade to CF, run AF80 preupgrade, run CF80 post upgrade; converting module records from AF to CF
AF 7.0.12	CF 8.0	AF upgraded to CF 8.0	Detect AF and upgrade to CF, run AF80 preupgrade, run CF80 post upgrade; converting module records from AF to CF
none	CF 8.1	New install of CF 8.1	Don't do anything related to AF; straight install CF 8.1
AF 7.0.x or lower	CF 8.1	AF upgraded to CF 8.1	Detect AF and upgrade to CF, run AF80 preupgrade, run CF80 post upgrade; converting module records from AF to CF; upgrade CF80 to CF81
CF 8.0	CF 8.1	CF 8.0 upgraded to CF 8.1	Don't do anything related to AF; upgrade CF80 to CF81

[johnhenley]

Group 2: ....

• [ ] Scheduler Entries (waiting on John)

These do not have an associated friendly name (just fully-qualified namespace/typename) so there is nothing to do this round. They will be replaced in future 8.1/8.2/9.0 by several different scheduler entries, and we will insert the correct friendly name at that time.

[johnhenley]

Okay. I look forward to hearing about what you find. :)

Stuff I found out:

1. Having a 'LegacyNames' node in the package manifest would likely have made this easier.
2. What triggers an upgrade of an existing module is having the existing package named in the manifest so we have to have a 'stub' package for 'Active Forums' for backward compatibility/support upgrades from 7.0 -> to future versions
3. Everything done related to determining which modules are installed is done at the time the package is read, so you can't tweak things during the install.
4. Packages in the manifest are read and processed in the order they are in the manifest, unless you use an installOrder attribute, which I'm adding since the 7.0 'stub' has to go first
5. Components in a package are read an processed in the order they are in the manifest, so if you put 'scripts' first, SQL is processed before the IUpgradeable, which is in the 'module' component. But IUpgradeable doesn't actually run until after app pool restart.
6. SQL scripts are processed in different order depending on type. If it's a new install, pre-upgrade scripts are skipped (expected). For a new install, you can have a version-agnostic install script (unexpected). If it's an upgrade, all pre upgrade scripts are executed first in order of appearance in the manifest and regardless of version (unexpected). Install/upgrade scripts are executed in order by version for newer version(s) to catch up. Last, any post-upgrade scripts are run (in order they appear in manifest regardless of version). (Unexpected). I would have expected the pre/install/post scripts to be woven together by version.
7. The SQL to swap in the different values in the packages and modules tables is way more complicated than a couple of UPDATE statements :)

[WillStrohl]

Can we avoid most of this in releases after 8.0 by using the UpgradeScript feature in DNN? It was in Brian Duke's DNN-Connect session. Did I send that to you already?

[johnhenley]

Can we avoid most of this in releases after 8.0 by using the UpgradeScript feature in DNN? It was in Brian Duke's DNN-Connect session. Did I send that to you already? In a nutshell, "no". (And yes, I saw his slides already, thanks for offering them!). The "upgrade" scripts are same as "install" scripts; the more-valuable ones are "preupgrade" and "postupgrade"--but only to a point. I'm using "preupgrade" in the AF package to capture the module/package records to a copy of the DNN tables. Then using those during UpgradeModule() to fix up the records in Packages/ModuleDefinitions/DesktopModules. It's the postupgrade that is troublesome. Because the install/commit is not yet done when the postupgrade script is run, the package/module records aren't quite ready. So, the technique I'm using is actually working fine, where I create the stored procedure normally in 08.00.00.SqlDataProvider, and then call it from UpgradeModule(), very much like you did. The difference is that backward compatibility to support AF 7 upgrades into the future beyond this version requires both having a stub AF7.0 package in the manifest, and some extra code in the 08.00.00 stored procedure. It's not awful, just awfully complicated.

[johnhenley]

oh yeah, and one more thing. The existing SQL objects. Because we have to have two packages in the manifest (one for 7.x backward compatibility) and second for 8.x+, once you install the upgrade, you only have one module, not two. So where do the SQL scripts go? If you put them with the 7.x package, upgrading to 8.00.00 works fine, but when you upgrade to 08.00.01, it tries to install the SQL objects associated with 7.x package. If you put them with 8.00 package, when you try to upgrade from 7.x to 8.x, it tries to install them again. There is no good solution. Except. I went through each and every SQL script back to the beginning and made them re-runnable. That way they can be associated with either package (I'm putting the 7.x scripts with the 7.x package, and the 8.x scripts with the 8.x package).

[johnhenley]

@WillStrohl After working on this the past few days, I'm inclined to suggest that we shelve the idea of renaming the package itself and just change the "friendly name" for this relationship. There are just too many hurdles and "smells" that have to be implemented to make it completely smooth and transparent. I have working 8.00.00 and 8.00.01 versions that upgrade smoothly and handle new installations, etc. and solve the scenarios enumerated above.

However, there are several things that have been required to get to this point:

1. Need to have two packages in the manifest to match old/new packages during an upgrade to accommodate a skip upgrade
2. Having the dual packages in the manifest effectively does an upgrade for one and a new install of the other one. For example, if you upgrade from 7.0.12 to 8.0.0, and later upgrade to 8.0.1, 7.0.12 gets reinstalled anew, and 8.0.0 gets upgraded to 8.0.1. To prevent duplicate sql object errors and schema changes required tweaking prior version sql files to handle a reinstall. The post-install cleans up the duplicated packages, but manipulating the sql for old versions makes me nervous.
3. To trigger the post-install requires that EVERY 8.0+ version into the future be in the VersionUpgradeList in the manifest. We can document and have comments, but it's likely to be missed at some point.

All of these introduce additional risk that needs to be weighed against the value of changing the package name.

Options/Ideas:

1. Just change the friendly name for now.

2. See if we can introduce a LegacyNames/LegacyName tag set into the manifest as part of the platform so that the installer code can match up old package name(s) and seamlessly upgrade them to a new package name. This will require time of course, someone to probably adopt it (hint, hint) as a pet feature, and if/when it's done, making that the minimum required version.

3. If you want, I can put in two PRs (or share the branches) for the 8.00.00 and 8.00.01 releases I have built so you can dissect and figure out a better way and/or see if you think it's a worthwhile approach.

4. Force a migration release, i.e. you must install 8.0.00 before anything beyond, so no skip upgrades. Acceptable but puts burden on administrators and likely introduces some support issues since you know how well people listen.

5. Maintain two parallel installers like DNN does for new installations vs upgrades and provide special instructions for the upgrade. Not sure if this really solves anything.

6. In my testing and experimenting, I felt like perhaps the sql object renaming alongside the package renaming felt like they go hand-in-hand and maybe that's the right time to do it?

Anyway, a ramble. Let me know what to think ...

[WillStrohl]

Wow... Excellent troubleshooting. I remember us doing this with the Media module and a couple of others many years ago, and I'm starting to remember some of these challenges you've highlighted. So, to make this work, it sounds like a SQL update could potentially make it work (option 6). I'm inclined for us to do that, but I'm thinking it's more trouble than it's worth...

Another issue with changing the package name is that the call out to report modules being installed, and subsequently visually notify people of an available upgrade would have problems too.

Let's skip changing the package name for now. Did you want to revert that in your own PRs, or should I?

[Timo-Breumelhof]

All done