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.

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

SQL Script fix for Portal Information #2522

Merged
merged 3 commits into from
Dec 31, 2018
Merged

SQL Script fix for Portal Information #2522

merged 3 commits into from
Dec 31, 2018

Conversation

mitchelsellers
Copy link
Contributor

Fixes #2521 by re-building the view for the PortalsDefaultLanguage.

@mitchelsellers
Copy link
Contributor Author

@dnnsoftware/tag I believe this is the right way to fix this, it is a breaking change though for anyone that has already tested a 9.3.0 upgrade, but I believe that would be the case no matter what. If I need to change let me know

@sleupold
Copy link
Contributor

I doubt that this is the proper fix. it looks like a data call in code behind is not matching columns properly, SQL is always matching by column names.

@valadas
Copy link
Contributor

valadas commented Dec 29, 2018

Well this would have been broken only on 9.3 and there was no production release of 9.3, so I think this is good in my opinion.

@mitchelsellers
Copy link
Contributor Author

Sebastian if the order changes of an underlying table a view built with Select * can get out of sync.

There are two fixes.

  1. Refresh the view
  2. Recreate the view

Given the prior processes in DNN I used option 2, since that is how we do it.

You can recreate this issue easily to confirm. We have also tested on multiple 9.3.0 builds that this resolves

@sleupold
Copy link
Contributor

@mitchelsellers
instead of recreating the view, you could exec sp_refreshsqlmodule '{databaseOwner}[{objectQualifier}vw_PortalsDefaultLanguage]', however, it would be better to generally avoid using "SELECT *" in views, see https://sqlblog.org/2009/10/10/bad-habits-to-kick-using-select-omitting-the-column-list, https://stackoverflow.com/questions/128412/sql-query-select-from-view-or-select-col1-col2-coln-from-view etc.

@mitchelsellers
Copy link
Contributor Author

@sleupold Yes, looking back at the history, execution of sp_refreshsqlmodule is not something we have done, and if an error occurs we wouldn't have a handle. Thus why I went the other route.

I left it as SELECT * for simplicity, if you want to update this PR to list all of the columns, go ahead. But I'm looking to keep the changes as minimal as possible for review etc at this point.

I'd like in 9.4, or 10.0 to review & remove all SELECT * usage across the board as a dedicated effort, etc.

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.

I have tested this solution to resolve the issue at hand.

Copy link
Contributor

@donker donker left a comment

Choose a reason for hiding this comment

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

... removed

Copy link
Contributor

@donker donker left a comment

Choose a reason for hiding this comment

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

Approving

@donker
Copy link
Contributor

donker commented Dec 30, 2018

BTW, to the point of using explicit columns or not: regardless of whether you do this, you need to recreate a view if you change the underlying table unless you're in a really specific use case. In my modules I use the * but use the views to retrieve the "super"-object than inherits from the base table object. Every time I add a column to the table, I need to rerun the view SQL. It's not very hard. When using explcicit columns you need to add the column to the view and also run this change. So I'm not losing any sleep over the approach we take in the future.

@valadas
Copy link
Contributor

valadas commented Dec 31, 2018

Cool, can someone merge then so we have a working nightly build.

@mitchelsellers mitchelsellers merged commit 69997b5 into dnnsoftware:development Dec 31, 2018
@mitchelsellers
Copy link
Contributor Author

Given that this already had the needed approvals, from not myself, I merged this change

@ohine ohine added this to the 9.3.0 milestone Jan 13, 2019
zyhfish pushed a commit to zyhfish/Dnn.Platform that referenced this pull request Mar 29, 2019
Fixes dnnsoftware#2521 by rebuilding the PortalsDefaultLanguage view
mitchelsellers pushed a commit that referenced this pull request May 14, 2019
* DNN-27517: force user logout after password changed in other place.

* DNN-27517: update code by review.

* DNN-27517: add host settings to control whether force logout after password changed.

* NOJIRA: mark as stable.

* Fixed bugs on add/remove user permissions for modules

* Change algorithm to SHA1CryptoServiceProvider

* Updated Issue Templates to include new RFC template and to support submissions for 9.3.0 release

* Corrected structure to avoid issue linking

* code review

* User registration: end the response after redirect (#2511)

* Initial New User Email Not Sending At Time of Creation (#2492)

This is alternative way to fix above issue proposed in dnnsoftware/Dnn.AdminExperience#174

As per @sleupold , we need to move email notifications from UI to core part.
Once this will be approved and merged, we can remove email notifications from UI and replace it with updated controller method to let notifications to be send to their recipients.

fixes #2424

* Fix for missing SQL change (#2522)

Fixes #2521 by rebuilding the PortalsDefaultLanguage view

* Resolve UserProfile Loading Errors From Unsecure pages (#2494)

* NOJIRA: mark as stable.

* DNN-21637: add config key.

* DNN-26576: prevent same-origin errors when loading popup and iframes from a secure page.

* code review

* Code review

* (DNN-10795) - All pages except home page return 404 (#2032)

* DNN-10795 - All pages except home page return 404

I have witnessed that occasionally on app pool recycle all,except the home page, will return 404 until the application pool is recycled a second time.

I've reviewed the code & believe that the root cause of the issue is due to the fact that the code that builds the tab index, portalDepths dictionary & tabPaths dictionary is not thread safe. I can see code in the method TabIndexController.FetchTabDictionary is using SharedDictionary classes to store the tab dictionaries, however the code is not thread safe when adding the dictionaries to the cache. Therefore when multiple threads are executing the FetchTabDictionary method it's possible for an empty dictionary to be added to the cache.

To resolve this issue the code has been updated so that only one thread can add the dictionaries to cache at a time.

* Updated comment to trigger Code Licence workflow.

* Added compiled DLLs that include the fix for bug DNN-10795 (All pages except home page return 404) for DNn versions 8.0.4 through 9.2.2

* Recursive read lock acquisitions not allowed (#2423)

* DNN-23293 Recursive read lock acquisitions not allowed in this mode.

* DNN-23293 Recursive read lock acquisitions not allowed in this mode.

* Performance problems when huge number of portal aliases is in use (#2514)

* DNN-27498 Performance Issues

* DNN-27498 Performance Issues

* minor formatting

* Fixed case sensitivity issue

* Added mixed cased alias support to unit tests

* Fixed VanityUrl unit tests

* Fixed broken LockStrategy unit tests (#2531)

* Delete Fixed-DLLs folder that was added as part of PR for bug DNN-10795. (#2535)

* Modules > ModuleCreator > fixed path error (#2527)

* Fixed issue in ModuleCreator > Web > template.ascx

* Update DNN Platform/Admin Modules/Dnn.Modules.ModuleCreator/Templates/Web/Module - HTML/template.ascx

Co-Authored-By: mean2me <emanuele.colonnelli@gmail.com>

* All languages are highlighted along with current
- add css for languages

* Log name of package when uninstalling extensions (#2557)

* remove spaces

* DNN-20856 After export with Content Localization site language flags disappears from pages (#2578)

* Fixed parallel build (#2562)

* Set active Nuget package source to All

* Fixed parallel build

* Inclusion of NDepend logo on the readme. (#2598)

* Fix for missing SQL change

Fixes #2521 by rebuilding the PortalsDefaultLanguage view

* Added attribution to NDepend for the usage of their ADO tooling

* Fix image/link markdown

* Get language from transferred parameter (#2607)

* switch encrypt method. (#2616)

* DNN-29484: switch encrypt method.

* NuGet Package Improvements

Changes to modernize the NuGet packages published by the DNN Platform, fixes #2586.  The below-submitted changes in structure have been validated by consultation with the DNN Platform Community, Microsoft Representatives, as well as validation of documentation per the published .nuspec file definition (https://docs.microsoft.com/en-us/nuget/reference/nuspec)

In detail, the following items have been changed:

* Migration of license information to the suggested <license> node rather than the deprecated <licenseurl> node.
* Inclusion of target framework for all included .dll files, this prevents installation of the package to pre-4.5 projects protecting downstream users.
* Improved package descriptions based on discussions held in the RFC regarding these improvements
* Added Package-to-Package dependencies to ensure quick usage and inclusion
* Updated the WebAPI and MVC packages to be holistic packages, including references to ALL needed items to develop using those patterns.

All changes are current for DNN Platform version 9.3.0 or later.  Packages have been built & tested locally with success.

## Suggested Usage

With these improved packages, development & references should be easier.

### MVC Modules

`Install-Package DotNetNuke.Web.Mvc`

Should be the only needed package installation.  It will install all needed dependencies, including the items necessary for WebAPI

### Modules Needing WebAPI (Not MVC)

`Install-Package DotNetNuke.WebApi`

Should be the only needed package for extensions not using MVC, however, needing to use WebApi for services.  This will work well for WebForms or Library projects, etc. that don't need the extra references for MVC/Razor

### WebForms/Limited Modules

`Install-Package DotNetNuke.Core`

The most simple modules, still using the WebForms pattern can use this package for the smallest footprint

For #2600

* Adjust the Source package to include changes from GitVersion (#2609)

* remove old ckeditor packaging steps

* Remove version to allow GitVersion to set it at build time (#2639)

* Adding 09.03.01.SqlDataProvider file

* Upgrade DNN to .NET Framework 4.7.2 (#2644)

* Upgraded app projects to .NET Framework 4.7.2; Added missing dependency to DotNetNuke.Tests.Core as it was missing DotNetNuke.Web.Client

* Removed targetframework web.config reference from Dnn.Modules.Console

* Reverted unintended changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants