Skip to content

Ensure new Localization HTTP module is running soon enough#4251

Merged
valadas merged 1 commit intodnnsoftware:release/9.8.0from
bdukes:fix/localization-xml-merge
Oct 27, 2020
Merged

Ensure new Localization HTTP module is running soon enough#4251
valadas merged 1 commit intodnnsoftware:release/9.8.0from
bdukes:fix/localization-xml-merge

Conversation

@bdukes
Copy link
Copy Markdown
Contributor

@bdukes bdukes commented Oct 27, 2020

Summary

This PR places the new Localization HTTP module right after URL Rewrite (which determines the culture from the URL), and then all modules after it can make use of that culture info. The current behavior puts it at the end for upgrades, which potentially causes issues.

It needs to be right after URL Rewrite (which determines the culture
from the URL), and then all modules after it can make use of that
culture info
@bdukes bdukes changed the base branch from develop to release/9.8.0 October 27, 2020 19:31
@bdukes bdukes added this to the 9.8.0 milestone Oct 27, 2020
Copy link
Copy Markdown
Contributor

@valadas valadas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, I'll do some localization tests on WebAPI as soon as possible too.

@valadas valadas merged commit e14db38 into dnnsoftware:release/9.8.0 Oct 27, 2020
@daguiler
Copy link
Copy Markdown
Contributor

Hi @bdukes
apparently this doesn't work because it creates a duplicate entry in the Web.config file.

Here's the error I got during install (installation cannot complete):

image

If this change will add the entry to the Web.config file, then maybe you need to remove it from the static files here and here.

valadas added a commit to valadas/Dnn.Platform that referenced this pull request Apr 14, 2025
I did a `git bisect` which pointed me to the commits in dnnsoftware#6038 to rollup the sql scripts for v10. This made no sense to me since it's not really related to web.config and @uzmannazari pointed out in dnnsoftware#6503 that a similar issue was caused by the LocalizationModule missing from the web.config. Also I could not reproduce the issue on upgrades. Then I thought since we have that v10 rollup, it uses the 10.00.00.config for the web.config xml merge scripts and up. Investigating the commit history of that file and comparing with 9.8.0 one I found that in dnnsoftware#4251, a bugfix was done to insure that the LocalizationModule is right after the UrlRewriteModule.

This means tha upon upgrades it would be in the right place but with clean installs, that would never run and it was also missing from the base web.config, thus making it not be there at all.

I was debating between putting this fix in the 10.00.00.config or just in the web.config I thought the web.config is fine as upon upgrades, the order would already have been fixed (as long as we document to upgrade to latest v9 before upgrading to v10). For clean installs it would just already be at the right place to start with.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants