-
Notifications
You must be signed in to change notification settings - Fork 751
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
Add Existing Module From Another Tab Relocates All Current Tab Modules to Same Pane #2289
Comments
Can anyone else confirm or acknowledge? This issue cripples building out sites with even only a couple of modules... I had to create my own quick and dirty razor module to work around this... |
please verify that the issue still persists in DNN 9.2.2, thank you |
Thank you for the follow-up, Sebastian. Yes, I can confirm that the issue persists in DNN v9.2.2 |
Thanks for the heads up, I'll try to reproduce. |
I can reproduce on Evoq Engage 9.2.0. I cannot reproduce in a completely clean Platform 9.2.2 install though. |
I can reproduce on a site upgraded from 9.1.1 to 9.2.2 |
And I can confirm it does not happen on a clean 9.2.2 install too. |
This also is happening on new install: |
@ohine @mitchelsellers I think it would be important to make sure this is taken care of for 9.3.0, can one of you guys add this to the milestone? |
Maybe related to #2163 , just writing this link here to check on the other issue when this one is resolved. It also happens on localized sites that use module references much more often that other users, it is a nightmare right now! I will try to take a look into the cause of this soon, if you have any clue of what changed, let me know... |
I am also experiencing this error and may not be able to wait until it is fixed by the community though I lack the knowledge to fix it myself. Could someone please recommend a course of action to resolve this issue with a workaround. For example, would some type of downgrade be possible? If so, is there a guide somewhere on how to downgrade DNN? |
@britchson there is no easy way to downgrade unfortunatelly, it is always a good idea to take a database backup and files backup before any upgrade for situations like this. That being said, this issue was tagged for a fix in Dnn 9.3.0 I am currently working on other parts of the next release but if nobody submits a pull request for this before I finish, it is definitely on my todo list. It is a big issue for me that have many localized sites. |
@valadas Thank you for your response. Unfortunately, the site was created at 9.2.1, so the issue has always existed. I'll wait then. |
Oddly enough, I am no longer able to reproduce, not sure why, tried enabling localization, different themes, etc. Not sure what makes it happen and not happen... |
So far, I have one site that was upgraded from 9.1.1 to 9.2.2 with localization and another site that was built on directly with localization also, those two have the issue. Trying to reproduce the same scenario 9.1.1>9.2.2 with localization and 9.2.2 new with localization and I can't make the issue happen again for some reason. |
I think we will leave this at v9.3.0 and make sure we get a few people test and report back how it's functioning against the rc build next week. |
IMO, A. We untag the milestone and we fix this when we finally can find step to correctly reproduce ( I have one production site with this issue but cannot find why it only happens on that one). B. We ask testers of 9.3.0 RC to focus on this and report back very detailed steps on how to reproduce. |
Cannot reproduce again on a Dnn8.0.4 to 9.3.0 upgrade and on another 9.3.0 clean install. |
I think this has to do with the script in the function _processModuleForDrag within the file ModuleDialog.js which is within the code for the EditBar. I believe that the function is attempting to create a collection of related modules so that those modules can be moved along with the module being dragged, by the code in the function dnn.ContentEditorManager.sortStop in the file ContentEditor.js. At line 902 in ModuleDialog.js the code compares the module IDs of the current module and other modules. If a module has a higher module id than the module being operated on the module will be added to the related modules collection and will be moved into the same pane as the module being operated on when the dnn.ContentEditorManager.sortStop function executes. I think the intention is that if there are multiple views (tabmodules with the same module id) for the same module on the page then all views should be transferred to the new page. However what happens instead is that all modules on the page with a higher moduleID are added to the related modules collection. Then all of those modules which were added to the related modules array are then moved to the new pane. Because the code is currently using a greater than operator in the if statement, only modules with a higher module Id would be considered related modules. Therefore the issue will only occur if you add a module from another page that has a lower module ID than the modules currently on the page. That may explain why @valadas was unable to reproduce the issue in some cases. To reproduce the issue add a module on one page. Then add a new module to a second page and then add the first module, from the first page, as a referenced module to the second page. The key is that the module being referenced must have a lower module id than the modules already on the second page. I'm not 100% sure what the original developer's intentions were with respect to related modules. Even if a there were multiple tabmodules for the same module on a page & the intention is to add them to the new page, I'm not sure that they should all be moved to the same pane if they weren't in the same pane already. If you comment out the push statement on line 905 of the file ModuleDialog.js the issue no longer occurs & other modules all stay in the correct panes. However I'm not sure this is the right solution because it seems that the code is there for a reason. I just don't understand exactly what it's meant to be doing. |
That is awesome info. I did not even know where to begin to look for that bug! Thanks. |
IMO we need to rethink, how we are storing data for modules "display on all pages". If they should appear on ALL pages (including admin), they are currently stored without additional tabmodule records for every page, Only after you moved or removed it on a single page, it will create all these links. |
@sleupold I agree this needs a rethink, almost every time I had clients use that feature on multiligual sites, we ran into problems. I try to avoid it as much as possible for that reason. For me, if it should show on all pages, it needs to be in the theme (skin) but I get that users need a way to show something on all pages easily. I will try to submit a PR to at least fix the issue at hand, but a rethink would be good in the future yes. |
@sleupold also, this is not with "show modules on all pages" but just "insert an existing module" just one time. |
@cameron-moore I can still not reproduce the issue, are those steps to reproduce correct?
If not, can you rephrase how you are able to reproduce in a list of steps. |
@valadas Yep, that's the steps I used to reproduce it. Below is a link to a video of me doing the same thing. Only difference was this time I added both pages at once, using the Add Multiple Pages option. However, I get the same result whether I add two pages ast once or add the first page and then add the second page later. Steps followed were:
Hope that helps. Let me know how it goes. |
I will look into this. |
thx Peter. Let me know if you need any help. |
In the meantime, you can apply the fix above. It's just 2 js files to modify. |
OK, thx, it's not a problem for me at the moment. I was just confirming your findings on 9.4.1. |
I have a client that has this issue, but on one of my test installs (9.3.2) I cannot reproduce it using these steps: |
Here are the step I can reproduce in 9.4.1 |
Can you guys check 2 things please for triage:
|
|
Thank you. |
By following these instructions I solved the problem: comment-552179624 thanks to jacoch |
@david-poindexter you marked the Effort as high, but I think the fix is in comment-552179624 ? |
I see this issue exists in 09.05.00 as well. I had thought it was fixed a couple releases ago? |
We did not receive a PR with the proposed changes, @jacoch can you submit a pull request for your changes please ? |
Well it was just a work around I used on my side. I thought it would be fixed by devs in another way. But sure, I can open a pull request. |
that would be really nice :-) |
Description of bug
When adding an existing module from another tab to the current tab, all of the current tab's modules move to the pane selected for the copied module.
Steps to reproduce
Current result
All existing modules move to the target pane of the copied module.
Expected result
Existing modules should remain in the same pane they were before adding the existing module
Affected version
Affected browser
The text was updated successfully, but these errors were encountered: