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

appsettings : ImageFileTypes Schema Vs Docs is wrong #14131

Closed
TRexStark opened this issue Apr 20, 2023 · 18 comments
Closed

appsettings : ImageFileTypes Schema Vs Docs is wrong #14131

TRexStark opened this issue Apr 20, 2023 · 18 comments

Comments

@TRexStark
Copy link

TRexStark commented Apr 20, 2023

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

10.5.0

Bug summary

I'm not sure if this has been raised before, but I think I've seen it somewhere and I have looked but could not find anything - so I apologise if it's a duplicate.

I wasn't sure if I should raise this here or in the Docs repo as they are both related and unsure which one is wrong (Docs Vs CMS).

It seems that the Imaging:ImageFileTypes is either using the wrong schema or the documentations may be wrong.

Specifics

Both Documentation for v10 and v11 outline an example as "ImageFileTypes": ["jpeg", "jpg", "gif", "bmp", "png", "tiff", "tif"] which is an array.

When setting this configuration in appsettings.json as per the docs, intelliSense is returning a warning as "Value must be one of the following types: string".

When inspecting appseetings-schema.json > line 923, you can see that the type for "ImageFileTypes" is set to string.

Steps to reproduce

  • Using Visual Studio
  • Make sure intelliSense is activated and properly setup
  • Add the below configuration to appsettings.json.

"Umbraco": { "CMS": { "Content": { ... "Imaging": { "ImageFileTypes": ["jpeg", "jpg", "gif", "bmp", "png", "tiff", "tif"], ... }, ... } } }

  • Notice that "ImageFileTypes" gets highlighted and check the warning message when you hover over it

UPDATE: The same goes for Plugins:properties:BrowsableFileExtensions

Expected result / actual result

either the schema and implementation or documentations need to be updated so we know what we should do.


This item has been added to our backlog AB#32240

@github-actions
Copy link

Hi there @TRexStark!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

  • We'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
  • If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
  • We'll replicate the issue to ensure that the problem is as described.
  • We'll decide whether the behavior is an issue or if the behavior is intended.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

@nikolajlauridsen
Copy link
Contributor

nikolajlauridsen commented Apr 20, 2023

Hey @TRexStark it looks like your schema may be a bit out of date, I'd recommend trying to delete it and rebuild, this should regenerate it, looking at my own test project the schema correctly specifies it as an array:

image

@TRexStark
Copy link
Author

Thanks @nikolajlauridsen.

Unfortunately, that didn't work for me.
Also that is not the reference that intelliSense is warning about, The issue for 'ImageFileTypes' is in line 923 under umbracoContent : properties : Imaging

I went to check the Generator in v10/main and the client referenced has the same issues outlined.

@elit0451
Copy link
Member

elit0451 commented Apr 21, 2023

I also experienced the same thing on VS, even though the IntelliSense suggest correctly the array type:

Intellisense

warning

I also tried regenerating the schema which worked until I reopened the project in VS again ..

It seems like in appsettings-schema.json we have defined ImageFileTypes twice:

"ImageFileTypes": {
  "description": "Collection of accepted image file extensions",
  "type": [
    "string"
  ]
}
"ImageFileTypes": {
  "type": "array",
  "description": "Gets or sets a value for the collection of accepted image file extensions.",
  "default": "jpeg,jpg,gif,bmp,png,tiff,tif,webp",
  "items": {
    "type": "string"
  }
},

@Zeegaan
Copy link
Member

Zeegaan commented Apr 24, 2023

I can reproduce this! Even in rider 🤯
image

Reproduction steps

  • Install the Umbraco 10.5 template with this CLI command dotnet new install Umbraco.Templates::10.5.0
  • Create a project dotnet new umbraco -n "TestSchema"
  • Go into your appsettings.json file and add the following code snippet in the Umbraco::Content section
  "Imaging": {
    "ImageFileTypes": []
  },

@github-actions
Copy link

Hi @TRexStark,

We're writing to let you know that we would love some help with this issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post.

Thanks muchly, from your friendly Umbraco GitHub bot :-)

@Zeegaan
Copy link
Member

Zeegaan commented Apr 24, 2023

I looked into this, and there are 2 ImageFileTypes
image
So it should be a simple one to look into why the schema generates 2 different ones in v10, but not in v11, which is why this is marked as up for grabs 🙌

@miguelcrpinto
Copy link
Contributor

miguelcrpinto commented Aug 5, 2023

This seems to happen as well in V10 for the RichTextEditor Plugins

"Umbraco": {
    "CMS": {
      "RichTextEditor": {
        "Plugins": [""]
      }
    }
  }

@miguelcrpinto
Copy link
Contributor

miguelcrpinto commented Aug 5, 2023

@Zeegaan I was able to reproduce the issue (both with the ImageFileTypes and the RichTextEditor Plugins) in V11.4.1

image

@miguelcrpinto
Copy link
Contributor

miguelcrpinto commented Aug 14, 2023

I believe I figured out the issue. The appsettings-schema.json is saying that both the https://json.schemastore.org/appsettings.json and the appsettings-schema.Umbraco.Cms.json apply. In the case of appsettings-schema.Umbraco.Cms.json, both the Imaging:ImageFileTypes and the RichTextEditor:Plugins are of type string array. But in the https://json.schemastore.org/appsettings.json both these properties are declared as strings.

If you remove the declaration of https://json.schemastore.org/appsettings.json from the appsettings-schema.json the issue is gone.

After closer inspection to https://json.schemastore.org/appsettings.json, it seems there are a lot of Umbraco specific properties there and because of this, I'm of the opinion that they should not be in a general appsettings.json schema definition. My suggestion would be to remove all umbraco specific properties from there and keep them in the appsettings-schema.Umbraco.Cms.json

The umbraco settings were added to the https://json.schemastore.org/appsettings.json in this PR to the schemastore

@miguelcrpinto
Copy link
Contributor

miguelcrpinto commented Aug 16, 2023

Have you considered moving the Umbraco settings out of the appsettings.json into a separate file (something like UmbracoSettings.json) with it's own schema?
This would keep the appsettings.json cleaner, would make it easier to find and maintain the umbraco settings in projects with lots of settings and you'd not need to maintain and ship the custom appsettings-schema.json anymore. The schema for this file (appsettings-schema.Umbraco.Cms.json) could either be included in the nuget or just published (versioned) in github.

A basic version of the file (with the schema available in the project) could look something like this:

{
  "$schema": "./appsettings-schema.Umbraco.Cms.json",
  "Umbraco": {
    "CMS": {
      "Global": {
        "Id": "<project id>",
        "UseHttps": true,
        "SanitizeTinyMce": true
      },
      "Content": {
        "AllowEditInvariantFromNonDefault": true,
        "ContentVersionCleanupPolicy": {
          "EnableCleanup": true
        }
      }
    }
  }
}

I can help implementing this change if needed.

@elit0451
Copy link
Member

elit0451 commented Aug 23, 2023

Hi @miguelcrpinto,

Thanks for the extra effort to track this down 🙌 H5YR!
We discussed this and we would make sure to remove Imaging::ImageFileTypes, RichTextEditor::Plugins and Plugins::properties::BrowsableFileExtensions, that were pointed out here, from the https://json.schemastore.org/appsettings.json 🙂 That way we will only have their string array definitions

@miguelcrpinto
Copy link
Contributor

@elit0451 considering that Umbraco is shipping with it's own schema (which depends on the CMS version) and including it in the appsettings-schema.json, shouldn't we just remove everything that is umbraco related from the https://json.schemastore.org/appsettings.json?

@elit0451
Copy link
Member

elit0451 commented Aug 23, 2023

Yes, that makes good sense!

@miguelcrpinto
Copy link
Contributor

I just created the Pull request to the schemastore: SchemaStore/schemastore#3162

Once this is merged, we need to test again but the issue should be gone after refreshing the schema definition in Visual Studio (code)

@miguelcrpinto
Copy link
Contributor

The SchemaStore PR has just been merged and it fixed the issue.

If you visit https://json.schemastore.org/appsettings.json there are no more references of Umbraco

You might need to force VS to reload the schemas (right click in the appsettings.json editor window):
image

@Zeegaan
Copy link
Member

Zeegaan commented Aug 28, 2023

@TRexStark could you confirm this is no longer an issue for you? Then we can go ahead and get this closed 🚀 😊

@TRexStark
Copy link
Author

Hi @Zeegaan it seems that this is no longer an issue. Thank you. I'll close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants