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

File Extension management - site wide. Fix for #3493 #3528

Merged
merged 4 commits into from
Dec 4, 2023
Merged

File Extension management - site wide. Fix for #3493 #3528

merged 4 commits into from
Dec 4, 2023

Conversation

leigh-pointer
Copy link
Contributor

I have added to the site settings file extension management. The Constants remain for backward compatibility.
If the extensions are not updated then the Constant will be used.

I have added to the site settings file extension management.
The Constants remain for backward compatibility.
If the extensions are not updated then the Constant will be used.
@leigh-pointer leigh-pointer changed the title File Extension management - site wide. File Extension management - site wide. Fix for #3493 Dec 1, 2023
@sbwalker
Copy link
Member

sbwalker commented Dec 2, 2023

@leigh-pointer one observation... in razor components you do not need to call SettingService.GetSiteSettingsAsync() in order to get Site Settings - they already exist in the PageState.Site.Settings property.

@sbwalker
Copy link
Member

sbwalker commented Dec 2, 2023

@leigh-pointer one other observation... the logic for using FileManager was very simple for developers in the past:

<FileManager FileId="@_logofileid" Filter="@Constants.ImageFiles" @ref="_logofilemanager" />

With the addition of a property to the Site model it could be:

<FileManager FileId="@_logofileid" Filter="@PageState.Site.ImageFiles" @ref="_logofilemanager" />

The ImageFiles property would be read only and would utilize the Site.Settings property internally to get the value.

This eliminates the need for developers to have to figure out how to get the value from Settings and would greatly simplify the code.

@leigh-pointer
Copy link
Contributor Author

@sbwalker i was wondering if there was a better way! Ok! I will action it!

@leigh-pointer
Copy link
Contributor Author

@sbwalker Fixed it

@sbwalker
Copy link
Member

sbwalker commented Dec 4, 2023

@leigh-pointer this looks good. There is only 1 remaining item:

When dealing with Settings it is very important to set the IsPrivate property properly:

settings = SettingService.SetSetting(settings, "ImageFiles", _ImageFiles, true);

The last parameter in this line of code is set to "true" which says that the ImageFiles setting is private. This means that unless the user is logged in as a site administrator (or host) this setting will be filtered from the Settings collection which is returned to the user. I do not believe this is the intended behavior, as this setting is used in the FileManager which is utilized in many module components which do not require the user to be an admin (ie. user profile, etc...). So I believe the IsPrivate property should be set to False.

To provide further clarification:
IsPrivate = false - return the setting so that it can be used in client-side scenarios
IsPrivate = true - filter this setting so that it is not available in client-side scenarios - UNLESS the user is an admin

@leigh-pointer
Copy link
Contributor Author

@sbwalker that is already being set. Settings are only saved from the Site component.
image

@sbwalker
Copy link
Member

sbwalker commented Dec 4, 2023

@leigh-pointer you are not understanding the behavior of the IsPrivate property. It has nothing to do with where settings are saved... it has to do with where settings are read. As an example, if you are a regular user and you browse to your user profile so that you can upload a photo... this uses the FileManager component. The FileManager component uses the ImageFiles setting to set the list of allowable file extensions which a user can upload. In this context (non-Admin) the ImageFiles setting will NOT exist in the PageSettings.Site.Settings collection. This is because it was created as a Private setting - which means that it should NOT be shared externally (ie. via the API). The only exception is when the user is an Administrator it will be in the collection because the Admin user is authorized to access it.

@sbwalker sbwalker merged commit 7550a19 into oqtane:dev Dec 4, 2023
1 check passed
sbwalker added a commit to sbwalker/oqtane.framework that referenced this pull request Dec 12, 2023
…leFiles if they are different than the default - otherwise it sets them to blank. This provides forward compatibility if the default values are updated in future framework versions.
sbwalker added a commit that referenced this pull request Dec 12, 2023
modify #3528 so that it only saves the ImagesFiles and UploadableFiles if they are different than the default - otherwise it sets them to blank. This provides forward compatibility if the default values are updated in future framework versions.
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.

2 participants