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

[Bug] PageState.Site.Settings when setting is an empty string value #4095

Closed
1 task done
vnetonline opened this issue Apr 2, 2024 · 15 comments
Closed
1 task done

Comments

@vnetonline
Copy link
Contributor

vnetonline commented Apr 2, 2024

Issue

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

When you have a scoped theme setting of type string and the value is an empty string for a Sitescope the PageState.Site.Settings is not updated.

Expected Behavior

The PageState.Site.Settings retains the old value, even if the setting value is updated to empty string.

Steps To Reproduce

  1. Add fowling HTML code to introduce Hero settings to ThemeSettings.razor of default Oqtane theme
<div class="row mb-1 align-items-center">
    <Label Class="col-sm-3" For="_footer" HelpText="Optionally enter hero section HTML markup for this page">Hero HTML:</Label>
    <div class="col-sm-9">
        <button type="button" class="btn btn-secondary" @onclick="LoadDefaultHero">Load Default Hero HTML</button>
        <textarea id="_hero" class="form-control" @bind="@_hero" rows="5"></textarea>
    </div>
</div>
  1. Add the following LoadDefaultHero Method with Get property to code part of ThemeSettings.razor
private void LoadDefaultHero()
{
    _hero = DefaultHero;
}

private string DefaultHero
{
    get
    {
        return
            "<section id=\"hero\" class=\"d-flex justify-content-center align-items-center\" style=\"width: 100%; height: 80vh; background: url('Themes/Amazing.Theme.Membership/assets/img/hero-bg.jpg') top center; background-size: cover; position: relative;\">\n" +
            "	<div class=\"container position-relative\" data-aos=\"zoom-in\" data-aos-delay=\"100\">\n" +
            "	  <h1>Learning Today,<br>Leading Tomorrow</h1>\n" +
            "	  <h2>We are team of talented designers making websites with Bootstrap</h2>\n" +
            "	  <a href=\"courses.html\" class=\"btn-get-started\">Get Started</a>\n" +
            "	</div>\n" +
            "</section>\n";

    }
}
  1. Add a string property to the Default Oqtane Theme ThemeSettings.razor called "_hero" and initialize the property as follows

    private string _hero = "";

  2. Modify the LoadSettings to the following code to load Hero setting

    private async Task LoadSettings()
    {
        if (_scope == "site")
        {
            var settings = PageState.Site.Settings;
            _login = SettingService.GetSetting(settings, GetType().Namespace + ":Login", "true");
            _register = SettingService.GetSetting(settings, GetType().Namespace + ":Register", "true");
            _footer = SettingService.GetSetting(settings, GetType().Namespace + ":Footer", "false");
            _hero = SettingService.GetSetting(settings, GetType().Namespace + ":Hero", "");
        }
        else
        {
            var settings = await SettingService.GetPageSettingsAsync(pageId);
            settings = SettingService.MergeSettings(PageState.Site.Settings, settings);
            _login = SettingService.GetSetting(settings, GetType().Namespace + ":Login", "-");
            _register = SettingService.GetSetting(settings, GetType().Namespace + ":Register", "-");
            _footer = SettingService.GetSetting(settings, GetType().Namespace + ":Footer", "-"); 
            _hero = SettingService.GetSetting(settings, GetType().Namespace + ":Hero", "");
        }
        await Task.Yield();
    }
  1. Updated the following code to update Hero setting based on scope
    public async Task UpdateSettings()
    {
        try
        {
            if (_scope == "site")
            {
                var settings = await SettingService.GetSiteSettingsAsync(PageState.Site.SiteId);
                if (_login != "-")
                {
                    settings = SettingService.SetSetting(settings, GetType().Namespace + ":Login", _login, true);
                }
                if (_register != "-")
                {
                    settings = SettingService.SetSetting(settings, GetType().Namespace + ":Register", _register, true);
                }
                if (_footer != "-")
                {
                    settings = SettingService.SetSetting(settings, GetType().Namespace + ":Footer", _footer, true);
                }
                settings = SettingService.SetSetting(settings, GetType().Namespace + ":Hero", _hero, true);
                await SettingService.UpdateSiteSettingsAsync(settings, PageState.Site.SiteId);
            }
            else
            {
                var settings = await SettingService.GetPageSettingsAsync(pageId);
                if (_login != "-")
                {
                    settings = SettingService.SetSetting(settings, GetType().Namespace + ":Login", _login);
                }
                if (_register != "-")
                {
                    settings = SettingService.SetSetting(settings, GetType().Namespace + ":Register", _register);
                }
                if (_footer != "-")
                {
                    settings = SettingService.SetSetting(settings, GetType().Namespace + ":Footer", _footer);
                }
                settings = SettingService.SetSetting(settings, GetType().Namespace + ":Hero", _hero);
                await SettingService.UpdatePageSettingsAsync(settings, pageId);
            }
        }
        catch (Exception ex)
        {
            await logger.LogError(ex, "Error Saving Settings {Error}", ex.Message);
            AddModuleMessage("Error Saving Settings", MessageType.Error);
        }
    }
  1. Run Oqtane
  2. Edit Homepage, go to Theme Settings tab, change the scope to Site,
  3. Load the Default Hero HTML, and save
  4. Go back to edit Homepage, go to Theme Settings tab, change the scope to Site
  5. Change the Hero setting to nothing, empty string
  6. You will see that the PageState.Site.Settings retains the Hero HTML previously entered

Screen Shots

UpdateSettings Method you can see hero is empty

image

LoadSettings method you can see that the Hero settings scoped to site retains its old value

image

Oqtane Info

Version - 5.1
Render Mode - Static

Anything else?

I think this bug is in the UpdateSiteSettingsAsync

image

Also i have noticed that when in static render mode the Sync Events are not firing

AddSyncEvent(setting.EntityName, setting.SettingId, SyncEventActions.Update);

@sbwalker
Copy link
Member

sbwalker commented Apr 2, 2024

@vnetonline I am fairly sure the bug is actually related to how the SetSetting() method is being called. The code:

settings = SettingService.SetSetting(settings, GetType().Namespace + ":Hero", _hero, true);

is setting the IsPrivate parameter to true. The IsPrivate setting indicates whether a setting should be accessible in the client UI or not. Note that when you are logged in as an Administrator, you will have access to ALL settings in the client UI because your user account has the appropriate permissions. However, when you log out and access the UI, any settings which have IsPrivate set to True will NOT be available. I believe what is happening is that the Site Settings are using the default values set in the GetSetting() method because the settings are not in the collection passed to the client.

Investigating further, it appears this problem exists in the ThemeSettings.razor of the default Oqtane theme - but only for the Site Settings - the Page Settings are fine because they do not specify the IsPrivate parameter when using SetSetting(). The default Theme template also has the problem.

@sbwalker
Copy link
Member

sbwalker commented Apr 2, 2024

Can you please explain what you mean by "in static render mode the Sync Events are not firing"... when I add a breakpoint to EventDistributorHostedService it is clear that events are firing:

image

@sbwalker
Copy link
Member

sbwalker commented Apr 2, 2024

The Settings issue appears to be resolved by #4098 (removing IsPrivate parameter from SetSetting call in theme)

@sbwalker
Copy link
Member

sbwalker commented Apr 3, 2024

@vnetonline were you able to verify the information I shared above? Does it resolve the problem you were experiencing? And can you please elaborate on the Sync Event comment?

@vnetonline
Copy link
Contributor Author

No it doesn't solve the problem I am having I will give you some more info today.

@vnetonline
Copy link
Contributor Author

vnetonline commented Apr 4, 2024

@sbwalker I have changed the code as per your suggestion

here is my UpdateSetting Method

image

here is my LoadSettings Method

image

When I change the scope to Site where the hero set is an empty string, the value is retained.

So it seems like when I hit save, the PageState.Site.Settings is not updated.

in regard to my comment regarding Syn Events can you please explain in the SIte Router the following code and why it only runs in Interavtive Render Mode

        if (PageState.RenderMode == RenderModes.Interactive)
        {
            // process any sync events (for synchrozing the client application with the server)
            var sync = await SyncService.GetSyncEventsAsync(lastsyncdate);
            lastsyncdate = sync.SyncDate;
            if (sync.SyncEvents.Any())
            {
                // reload client application if server was restarted
                if (sync.SyncEvents.Exists(item => item.Action == SyncEventActions.Reload && item.EntityName == EntityNames.Host))
                {
                    NavigationManager.NavigateTo(_absoluteUri, true);
                    return;
                }
                // refresh PageState when site information has changed
                if (sync.SyncEvents.Exists(item => item.Action == SyncEventActions.Refresh && item.EntityName == EntityNames.Site && item.EntityId == SiteState.Alias.SiteId))
                {
                    refresh = true;
                }
            }
        }

@sbwalker
Copy link
Member

sbwalker commented Apr 4, 2024

@vnetonline your UpdateSettings method contains conditional logic for the _hero field which only sets its value if it is not equal to empty string. As a result, if the value is set to empty string in the UI, the value will never be set in the database. You should remove the conditional check for the _hero field. You can see an example here:

https://github.com/oqtane/Oqtane.Theme.Arsha/blob/main/Client/Themes/ThemeSettings.razor

In regards to SyncEvents and Render Modes...

In Interactive render mode the application is a SPA and retains state for the user session. Each time the Url changes the router uses the state which was already loaded into memory and does not reload from the server. When another user modifies information on the server (ie. changes site settings, add new pages, adds new modules, etc... ) the user session state needs to be refreshed. The SyncService raises Refresh or Reload events to the router to let it know that that state needs to be reloaded from the server. This is documented in this blog: https://www.oqtane.org/blog/!/53/oqtane-server-events

In static render mode the application is a server-based app and is stateless (ie. there is no user session). Each time the Url changes it reloads information from the server. When another user modifies information on the server (ie. changes site settings, add new pages, adds new modules, etc... ) the existing user will get the updated information immediately upon the next Url change. There is no need for a SyncService in this scenario.

You indicated "in static render mode the Sync Events are not firing" which seems to suggest you are experiencing a problem... can you please explain further.

@vnetonline
Copy link
Contributor Author

Thanks for the explanation, even after removing the conditional logic it doesn't work because PageState.Site.Settings doesn't get updated. It holds the old value works fine for the dropdowns but not when there is a setting with empty strings

The SyncEvents are working fine it was just my misunderstanding on how they work

@vnetonline
Copy link
Contributor Author

vnetonline commented Apr 5, 2024

Now my UpdateSetting method looks like this

    public async Task UpdateSettings()
    {
        try
        {
            if (_scope == "site")
            {

                var settings = await SettingService.GetSiteSettingsAsync(PageState.Site.SiteId);
                if (_login != "-")
                {
                    settings = SettingService.SetSetting(settings, GetType().Namespace + ":Login", _login);
                }

                if (_register != "-")
                {
                    settings = SettingService.SetSetting(settings, GetType().Namespace + ":Register", _register);
                }

                if (_breadcrumbs != "-")
                {
                    settings = SettingService.SetSetting(settings, GetType().Namespace + ":Breadcrumbs", _breadcrumbs);
                }

                settings = SettingService.SetSetting(settings, GetType().Namespace + ":Hero", _hero);
               
                await SettingService.UpdateSiteSettingsAsync(settings, PageState.Site.SiteId);

            }
            else
            {
                var settings = await SettingService.GetPageSettingsAsync(pageId);
                if (_login != "-")
                {
                    settings = SettingService.SetSetting(settings, GetType().Namespace + ":Login", _login);
                }

                if (_register != "-")
                {
                    settings = SettingService.SetSetting(settings, GetType().Namespace + ":Register", _register);
                }

                if (_breadcrumbs != "-")
                {
                    settings = SettingService.SetSetting(settings, GetType().Namespace + ":Breadcrumbs", _breadcrumbs);
                }

                settings = SettingService.SetSetting(settings, GetType().Namespace + ":Hero", _hero);
                
                await SettingService.UpdatePageSettingsAsync(settings, pageId);

            }
        }
        catch (Exception ex)
        {
            await logger.LogError(ex, "Error Saving Settings {Error}", ex.Message);
            AddModuleMessage("Error Saving Settings", MessageType.Error);
        }
    }

These are the settings in my DB

image

So the update is working fine, but when you load the settings by changing the scope to Site I still have Site Setting populated with HTML, which is not in the DB because PageState.Site.Settings is not updated.

@sbwalker, you state that when RenderMode is set to Static that the PageState is reloaded "Each time the Url changes" can you point me to where this happens in the SIte Router??

image

@sbwalker
Copy link
Member

sbwalker commented Apr 5, 2024

@vnetonline I cannot reproduce your issue. I modified the ThemeSettings to allow entry of a text field... and logic for loading/saving the text field. And I added logic to the Default theme component to display the information:

Private.-.Default.Site.Firefox.Developer.Edition.2024-04-05.15-02-31.mp4

(ignore the black highlighting of the Footer field in Theme Settings - it is because of other CSS testing I am doing).

Perhaps you are having problems because the setting was declared as Private initially and then you changed it to be Public? Try changing the name of the setting so that it is treated like a brand new setting that has never been referenced before.

In regards to your questions about PageState, when you navigate to any link in Static render mode it sends a request to App.razor on the Server which executes/renders all components which are required for that route. If you put a breakpoint on the Refresh() method in SiteRouter it will be hit on every navigation (note that the LocationChanged will NOT be invoked because this is only applicable in Interactive render mode).

@vnetonline
Copy link
Contributor Author

@sbwalker My use case is a bit different

I have a scoped setting say if you want to set up a hero image as a site setting so it appears on every page and then some pages have a different image and text so that would be set in the Page scope. If the page scope is empty it would default to the site image

This works great for dropdown which always have a value true or false but it doesn't work for empty string.

@sbwalker
Copy link
Member

sbwalker commented Apr 6, 2024

@vnetonline you did not explain any of your specific business rules - you just indicated that site settings were not saving properly… so it’s very difficult to diagnose a problem when the details are not fully explained.

@vnetonline
Copy link
Contributor Author

vnetonline commented Apr 6, 2024

@sbwalker yes you are right I should have explained better ... but if you look at my initial issue I specifically say scoped and detail how to reproduce the issue.

The issue may be with MergeSettings method possibly.

@vnetonline
Copy link
Contributor Author

vnetonline commented Apr 6, 2024

Because I can't resolve or find why the PageSate.Site.Settings doesn't get updated, I have had to work around the issue.

So in my Default.razor In my theme, I retrieve settings from DB that which are scoped for Page and Site.

protected override async Task OnParametersSetAsync()
    {
        try {

            var siteSettings = await SettingService.GetSiteSettingsAsync(PageState.Site.SiteId);
            var pageSettings = await SettingService.GetPageSettingsAsync(PageState.Page.PageId);
            var settings = SettingService.MergeSettings(siteSettings, pageSettings);
            _login = bool.Parse(SettingService.GetSetting(settings, GetType().Namespace + ":Login", "true"));
            _register = bool.Parse(SettingService.GetSetting(settings, GetType().Namespace + ":Register", "true"));
            _breadcrumbs = bool.Parse(SettingService.GetSetting(settings, GetType().Namespace + ":Breadcrumbs", "true"));
            _hero = SettingService.GetSetting(settings, GetType().Namespace + ":Hero", "");

        }
        catch
        {
            // error loading theme settings
        }
    }

NOTE: When Loading and Updating settings, I don't use PageState.Site.Settings instead II retrieve the setting from the DB

    public async Task UpdateSettings()
    {
        try
        {
            if (_scope == "site")
            {

                var settings = await SettingService.GetSiteSettingsAsync(PageState.Site.SiteId);
                if (_login != "-")
                {
                    settings = SettingService.SetSetting(settings, GetType().Namespace + ":Login", _login);
                }

                if (_register != "-")
                {
                    settings = SettingService.SetSetting(settings, GetType().Namespace + ":Register", _register);
                }

                if (_breadcrumbs != "-")
                {
                    settings = SettingService.SetSetting(settings, GetType().Namespace + ":Breadcrumbs", _breadcrumbs);
                }

                settings = SettingService.SetSetting(settings, GetType().Namespace + ":Hero", _hero);
               
                await SettingService.UpdateSiteSettingsAsync(settings, PageState.Site.SiteId);

            }
            else
            {
                var settings = await SettingService.GetPageSettingsAsync(pageId);
                if (_login != "-")
                {
                    settings = SettingService.SetSetting(settings, GetType().Namespace + ":Login", _login);
                }

                if (_register != "-")
                {
                    settings = SettingService.SetSetting(settings, GetType().Namespace + ":Register", _register);
                }

                if (_breadcrumbs != "-")
                {
                    settings = SettingService.SetSetting(settings, GetType().Namespace + ":Breadcrumbs", _breadcrumbs);
                }

                settings = SettingService.SetSetting(settings, GetType().Namespace + ":Hero", _hero);
                
                await SettingService.UpdatePageSettingsAsync(settings, pageId);

            }
        }
        catch (Exception ex)
        {
            await logger.LogError(ex, "Error Saving Settings {Error}", ex.Message);
            AddModuleMessage("Error Saving Settings", MessageType.Error);
        }
    }

    private async Task LoadSettings()
    {
        if (_scope == "site")
        {

            var settings = await SettingService.GetSiteSettingsAsync(PageState.Site.SiteId);
            _login = SettingService.GetSetting(settings, GetType().Namespace + ":Login", "true");
            _register = SettingService.GetSetting(settings, GetType().Namespace + ":Register", "true");
            _breadcrumbs = SettingService.GetSetting(settings, GetType().Namespace + ":Breadcrumbs", "true");
            _hero = SettingService.GetSetting(settings, GetType().Namespace + ":Hero", "");

        }
        else
        {

            var settings = await SettingService.GetPageSettingsAsync(pageId);
            _login = SettingService.GetSetting(settings, GetType().Namespace + ":Login", "-");
            _register = SettingService.GetSetting(settings, GetType().Namespace + ":Register", "-");
            _breadcrumbs = SettingService.GetSetting(settings, GetType().Namespace + ":Breadcrumbs", "-");
            _hero = SettingService.GetSetting(settings, GetType().Namespace + ":Hero", "");

        }
        await Task.Yield();
    }

@sbwalker
Copy link
Member

sbwalker commented Apr 8, 2024

since you found a way to resolve your problem, I am going to close this issue

@sbwalker sbwalker closed this as completed Apr 8, 2024
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

No branches or pull requests

2 participants