oqtane / oqtane.framework

CMS & Application Framework for Blazor & .NET MAUI
http://www.oqtane.org
MIT License
1.88k stars 543 forks source link

Bug: Updating Module Title Twice Does Not Save Title Changes - 5.1.0 #3977

Closed thabaum closed 7 months ago

thabaum commented 7 months ago

ISSUE

Description

The module title is not being updated once you set it, and try to reset it, without refreshing the page to get a fresh UI instance.

Steps to Reproduce...

  1. Edit page module title in page settings
  2. Save changes.
  3. While still on that page and most likely still in edit mode try to edit and save the title again
  4. Notice the title is still the same and does not get updated.

Happens both static and interactive Oqtane render modes.

I noticed in static at least that the carrot icon becomes inactive as well at times.

image

Not sure if the above issue with inactive dropdown actions is limited to my installation. It is something to be aware of while testing solutions.

zyhfish commented 7 months ago

it looks like the data is not synced between different db context instances, need to analyze further to find out the root cause.

thabaum commented 7 months ago

could of been fixed by #3979 or a recent update in visual studio. I am not experiencing this issue any longer with today's branch.

zyhfish commented 7 months ago

Hi @thabaum , this is still able to be reproduced with the latest code, could you please reopen the ticket so that it can be tracked? thx

thabaum commented 7 months ago

@zyhfish hello, I will try to reproduce again.

thabaum commented 7 months ago

@zyhfish Hello, I was unable to reproduce this on current dev branch. Did you test this with a hard refresh of the browser for the site page tested using Ctrl + F5 as I believe I did? Maybe this makes the difference? Thank you. Cheers!

Also be sure to update visual studio to latest sometimes that one gets me as I did update recently as well.

zyhfish commented 7 months ago

Hi @thabaum , it's able to be reproduced in the interactive render mode.

thabaum commented 7 months ago

@zyhfish hello, thanks for sharing your finding as I was testing mainly 5.1.0, but did notice this had the issue in both render modes.

Seems like when I switch to Interactive mode the issue was only the inactive drop down carrot icon after first save, but after refreshing page once everything works great again. image

I can reproduce this issue, it happens whenever you switch from static to interactive is through being in Server component interactivity mode image

You can even change pages and are unable to access module settings. And if you can access the module settings updating the page name the second time does not save to the database.

This issue to reproduce needs to be Rendermode: Interactive and Interactivity: Server as I don't think it has an issue after refreshing on "Auto" or an issue at all currently now in static SSR, but I have not tested all the different interactivity modes yet in static. I think this interactivity setting is something to review.

With Auto rendering mode enabled after a refresh things seem to have worked. Switching render modes to interactive does in fact still trigger this issue.

zyhfish commented 7 months ago

Hi @thabaum , the issue should be caused by #ed7904b67324326d00a86d645ae46ff01c1cb1c6 or #dcc8043cf60751e35e78579e749ca1bd4ce76a60. here is the screencast of the issue: https://drive.google.com/file/d/1nhFKDMZbZj2rrHU8aGAdaACtDxxAko4i/view?usp=sharing

thabaum commented 7 months ago

image

This renders the entire drop down inactive, no links do anything, except you can open the manage settings and click save, but nothing is saved.

In other words you cannot even select to delete the module.

thabaum commented 7 months ago

Closing as fixed in #3988

sbwalker commented 7 months ago

reopening as the standard process is that the issue is closed once the PR is reviewed, accepted, and merged

thabaum commented 7 months ago

I had thought this was fixed again as it was working in interactive mode for me with where I was at testing these other issues, so I switched to Static/Server UI component mode from interactive to verify it works in both modes and the issue could be reproduced.

If the fix is something else other than #3988 this should leave some added clues. Seems like switching render modes may trigger the issue as well.

sbwalker commented 7 months ago

@thabaum I am unable to reproduce this issue in either Static or Interactive render modes. So I am not going to merge PR #3988 unless the problem can be reliably reproduced.

zyhfish commented 7 months ago

Hi @sbwalker , I have attached the reproduce screencast in my previous comment, it can be reproduced with Interactive/Server render mode.

After deeper analyze the code, I have identified the root cause of this issue, it's related to the db context factory changes, we also need to use db context factory in the repository classes as well. please check whether PR #4006 is ok to be merged, after this I will submit another PR to make the same changes in other repository classes as well.

sbwalker commented 7 months ago

@zyhfish during the MVP Summit this week I asked the Blazor Product Team what the latest recommendation is for EF Core data access for Blazor (as the guidance has changed many times over the years ie. Scoped, Transient, OwnerComponentBase, etc…). I was told that DbContextFactory is the recommended approach.

So I think we should transition all Repositories to use DbContextFactory. The references to DbContext can be removed from the Repositories. However the service registration for DbContext in Startup must be retained for backward compatibility as custom modules will continue to rely on it (until they transition to DbContextFactory). The module template should be updated to use DbContextFactory as well. Do you have time to make these changes?

The reason why this issue is only coming to the surface now is that it’s a rare combination of events which can trigger it. Basically you need a long-lived component which is doing multiple data access calls to the same DbContext. And the component must be using a server-based rendering approach. So it’s quite rare - however it’s still a problem which can be easily avoided with DbContextFactory.

zyhfish commented 7 months ago

Hi @sbwalker , sure, I will work on this and submit a PR later. please check whether #4006 can be merged, or I can include it in the next PR with all other repository classes together.