umbraco / Umbraco.Marketplace.Issues

Public issue tracker for Umbraco Marketplace
2 stars 0 forks source link

Why is there a minimum of 1 item required for "AddOnPackagesRequiredForUmbracoCloud"? #29

Closed hfloyd closed 1 year ago

hfloyd commented 1 year ago

This is in the schema for the marketplace json: image

But it seems to me that the majority of packages will not require any Umbraco Cloud add-on package... My file fails schema validation if I have this:

"AddOnPackagesRequiredForUmbracoCloud": [],

But I feel this should be a valid entry....

If I update it to:

"AddOnPackagesRequiredForUmbracoCloud": [
    "NONE"
  ],

It passes validation, but since "NONE" is not a valid package ID, I'm not sure that is really what you want to be getting...

AndyButland commented 1 year ago

My thinking here was that if you don't need to add any information for an entry, you just omit it. But if you do, there should be at least one item or there's no point in having it in the file.

So here you can just omit the AddOnPackagesRequiredForUmbracoCloud entry if you don't have anything to add.

However now you point it out, I don't see there's any harm in having an empty array set here, so I'll remove the minItems setting.

hfloyd commented 1 year ago

Thanks, @AndyButland , I am dynamically generating the JSON, and using a Data model which is then serialized, so it's a bit trickier to just "exclude" the generation of that JSON property if it's empty. 😊

abjerner commented 1 year ago

JSON.net can be a little annoying regarding empty arrays and lists. For my own models, I ended up adding these methods (which JSON.net listens for):

https://gist.github.com/abjerner/19767bb967819be301182d7bddbce3ab#file-marketplacedetails-cs-L99-L129

Not exactly the prettiest solution (the methods have to be public), but it does the job.

Updating the schema to allow zero-length arrays sounds like a good solution πŸ‘

hfloyd commented 1 year ago

I see that an empty array now validates, thanks, @AndyButland ! My file still isn't getting picked for some reason... 😒 But I'm out of ideas.

nathanwoulfe commented 1 year ago

@hfloyd if you want to point me to your JSON file, I can have a look.

hfloyd commented 1 year ago

That would be so kind of you, @nathanwoulfe . Take a look here: https://dragonflylibraries.com/umbraco-marketplace-dragonfly.umbraco10.schemaimporter.json for NuGet package https://www.nuget.org/packages/Dragonfly.Umbraco10.SchemaImporter/

AndyButland commented 1 year ago

I ran this import locally to check, and found it was tripping up on a duplicate value provided in the AlternatePackageNames collection (which fails on import due to a unique constraint). I'll amend the importer to de-duplicate here.

AndyButland commented 1 year ago

Looks to be presented now: https://marketplace.umbraco.com/package/dragonfly.umbraco10.schemaimporter

hfloyd commented 1 year ago

Thanks so much! It looks great😊 Now I've got to update all my other packages 😁