Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ensure new Localization HTTP module is running soon enough #4251

Merged
merged 1 commit into from
Oct 27, 2020
Merged

Ensure new Localization HTTP module is running soon enough #4251

merged 1 commit into from
Oct 27, 2020

Conversation

bdukes
Copy link
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.

@bdukes
Ensure Localization HTTP Module placement
0406e13 
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
valadas
valadas approved these changes Oct 27, 2020
View reviewed changes
Copy link
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
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.

@daguiler daguiler mentioned this pull request Oct 29, 2020
Closed
@bdukes bdukes mentioned this pull request Oct 29, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@valadas valadas valadas approved these changes

@mitchelsellers mitchelsellers mitchelsellers approved these changes

Assignees
No one assigned
Labels
Type: Bug
Projects
None yet
Milestone
9.8.0
Development

Successfully merging this pull request may close these issues.
4 participants
@bdukes @daguiler @mitchelsellers @valadas