oqtane / oqtane.framework

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

Bug: User Profile Image Not Showing #3569

Closed thabaum closed 10 months ago

thabaum commented 10 months ago

The user profile image source attribute is resolving with an api request instead of the profile url.

The if statement checks for photo but then does not use photo to get the url.

I have a PR that resolves this and maybe acceptable I will submit in a minute.

sbwalker commented 10 months ago

Please provide more details on how to reproduce ie. which type of user did you add a photo for, what type of photo, etc... include a screen shot... etc...

sbwalker commented 10 months ago

Cannot reproduce locally with host account:

image

thabaum commented 10 months ago

@sbwalker you know I thought I had it working too... then it quit but I cant verify it was working as I was not paying that close of attention getting used to the behavior of profile being saved.

Now I have another issue of file extensions not allowed when trying to upload another image.

I am using one fairly large say 500px x 500px png image for logo and profile image I am testing things out with, uploaded to the places as discussed which brought upon the relating issues I have experienced since working in this area.

I checked the settings for allowed extensions and both image and file options where blank.

I set just the file upload to include png in the field first time looking at this area of that feature in the admin control panel and after setting the file (image did not work) property I was able to upload this image. I feel since this an image upload the image allowed extensions would be taken into consideration as well making you set them in both file and image allowable extensions or just the image extensions allowed properties or what is the point of the image extensions allowed property in site settings?

I will get back to you further on this once I figure it out. I may start a new installation from dev branch as this is not related to the last release.

On another note it is a pain to try to think of recovering all of these extensions Oqtane allowed by default in the event they get erased on a save. It would be nice to have an option to set app defaults link or a list of them where you can copy them from the tool tip for a quick reference in the event you lose them all, not that has ever happened except this once. It is odd I was able to upload earlier and then these settings removed.

I did create some other tenants on this installation that maybe bugging my solution instance out. I don't recall doing anything to delete these extension settings so it would be nice to know how that happened.

sbwalker commented 10 months ago

@thabaum you did identify an issue with the File Extensions. Based on the discussion in the Developers Meeting the implementation was changed so that the Site Settings values were only set if they were customized from the default settings. This provides forward compatibility if the default settings are changed. However, it appears that once the Site Settings are set to blank, then blank will always be returned rather than the default value (because blank is an actual value). This will need to be investigated further.

thabaum commented 10 months ago

@sbwalker A few odd things going on in this area i will test out further tomorrow, I am not sure if maybe the API is not getting the image since it is not an allowed extension, and I tested out this and yes in fact this is what caused this issue.

To create the issue hit save on the site settings admin panel so the allowed extensions are saved blank.

Then revisit the user profile.

The file upload will not upload and the image is not allowed to display anymore for allowed images.

If uploading a profile image it should be not allowing with a warning if not allowable media extension and upload extension. if i upload a text file from image file manager for profile photo it give a system error after allowing txt to be uploaded.

It is looking for png,jpg but I switched to see all files and tested uploading txt. It should generate a user error that wrong format or type or size.

File Manager should know you are looking for an image type and set these in parameters of component to avoid the error.

So in fact the saving of the extensions with having empty strings for these properties results in losing the default enums. This does cause the issue I am describing as a bug after further investigating and setting the image allowable extensions brings back the profile image.

I think we may need to have an override option or just run with suggested defaults. I would be more interested in removing some than adding any, but I dont want to lose them all and there needs to be a way to get the defaults from the enums to set these allowable extensions. Great enhancement just needs fine tuned.

leigh-pointer commented 10 months ago

@sbwalker I have this PR ready if you think this is viable a solution, adding a reset button to the UI.

Untitled video - Made with Clipchamp

sbwalker commented 10 months ago

@leigh-pointer I do not believe this should be handled in the user interface, as the main question a user would have (without the context of this thread) is "why would I need to reset the list of file extensions?". #3574 resolves the problem by only loading/saving the file extension settings if they are not null or empty - otherwise the setting value will always be returned as the system constant. This way if the system constant changes in the future, the site will inherit the changes.

sbwalker commented 10 months ago

@thabaum thank you for identify the issue with the file extensions. I am not inclined to make any other changes to the user profile behavior at this point - since the new behavior is now consistent with Login / Register. At some point in the future there will need to be an option for a public viewable user profile (currently only a registered user can view their own profile) - which may require some additional options/behaviors.

thabaum commented 10 months ago

@sbwalker you are welcome for discovering this extension issue and great to hear this issue was fixed.

Another issue is if you include spaces after , it wont work. So comma separated values without spaces should be included in the tool tip I believe to help or spaces trimmed on save (preferred).

There is yet another issue to still resolve in User Profile as well.

  1. Upload a file with an extension not allowed in user profile
  2. error is throw and file manager component is no longer available until refresh
  3. expected behavior is user being told this file extension is not allowed.
  4. file manager needs a mode or something to let it know to only allow certain extensions and not allow the upload unless it is allowed.
  5. to test add "txt" upload a text file to the user profile image area. The upload completes but then the file manager errors out. Potentially trying to display the text file. I wanted to break it somehow as a "new administrator" and this was successful at doing it.

So 2 additional issues with this enhancement discovered. The two I am discussing above I believe should be addressed immediately.

To Recap Issues From This Discussion of Issues:

  1. Save Behavior Issue: Resolved as not an issue and is expected and desired behavior. It is inline with most all other app behavior observed.

  2. File/Image Extensions Issue: Potentially resolved but I would like to add my 2 cents: Allowed extensions fields should be populated an admin can edit which they want to allow (blank for none, cant get more secure right? and Oqtane still functions so after setting up an application maybe you want to secure it further since nothing gets uploaded after a point, this gives a way to disable uploads).

  3. File Extension Field Trimming: Remove the spaces from the comma separated values on save. Quick fix as I should be able to do this PR. Naturally as a user you type a space after a comma, so I tested that knowing I would want it to work both ways for my end users experience.

  4. Profile Photo Upload Issue: If you upload a text file or other allowable file other than image the file manager critical error instead of a validation message.

  5. Public User Profile Issue: Currently there is no public facing user profile page and is to be added in the future. Enable/Disable option like Enable/Disable registrations. This should probably be a new issue brought up to give time to discuss how this should be implemented for feedback.

  6. SEO Friendly URLs Issue: It would be powerful for these profile pages and mod or user files to be SEO friendly. http://localhost:44357/api/file/image/3/400/400/crop/center/transparent/0/False is not very friendly as I preferred the results from the photo.Url property. If this was public facing it would be better to use a more friendly name and maybe add some meta to it. An example is having the photo displayed publicly as http://localhost:44357/user/unique-display-name/profile/profile_photo.jpg or http://localhost:44357/unique-display-name/profile/profile_photo.jpg depending on how you want to have these displayed. Some flexibility would be nice.

I figure a few of these will get fixed without creating issues we can discuss more details. I think issues should be created for these to marinate over time with feedback and keep them on our radar. It would be of value to developers, users and contributors to know some features moving up on the list like the public facing user profile feature not currently present to be included down the road. I almost think now is a time to add wish list items like this that are acceptable ideas. Get the ideas posting up in the discussion forum, accepted ones create an issue here to discuss in more detail how to include it.

This issue that in fact was not an issue turned more into a type of train wreck of other issues that just happen to relate to this area. If I didn't test this out this would of not been known and some reason I just happen to be working this direction. One thing led to another.

The user profile area was a great place to test out all the functionality of the new enhancement @leigh-pointer introduced enabling editing allowed file extensions in Oqtane Framework from Site Settings in the Admin Control Panel. However it also allowed me to stress test the File Manager component now that Oqtane Framework has this enhanced flexibility allowable file extensions and image extensions. A one up would be to have docs, video and audio extensions to cover all the different types of common media and documents allowed to upload for verification validation user input and for viewing or sharing these types of files with these extensions.

Pretty much when you open that section up in Site Settings you have 2 fields. I am thinking it should be broken into two categories or sub-sections. The "Global Upload Extensions" so to speak allowable list which includes all types you want to allow and currently exists, and then the "API Accessible Extensions" broken down into properties relating to documents and media which there is already one for images allowed. I am not sure what to call these exactly as these are just category or sub-section name examples by use case to help explain why they are different as the Allowed Image Extensions only works currently with the image handler API feature as it removes the image from profiles if you do not include it which is what gave the profile photo issue once this field was set blank or removed the extension that was allowed as an upload.

Phew... did I get all that out in one breath?! I tried... and I deleted half of it. I hope half of what I left makes sense!

Thanks again everyone for addressing the issues as you can and explaining things like the save profile return url behavior.

Cheers!

sbwalker commented 10 months ago
  1. Save Behavior Issue: resolved
  2. File/Image Extensions Issue: the extensions feature is intended to be a filter - not a way to completely disable file uploads. Fully disabling file uploads for ALL users (including host users) would have many impacts on usability.
  3. File Extension Field Trimming: - fixed here #3577
  4. Profile Photo Upload Issue: the system is not going to prevent an administrator from specifying a non-image file type in the Image Files extensions. As I mentioned when this feature was originally discussed - if Administrators expose their installation to risk by specifying unsecure or illogical file extensions, it is not the framework's responsibility.
  5. Public User Profile Issue: not in scope at this time
  6. SEO Friendly URLs Issue: not in scope at this time