Simplify and correct quality profile module format

[francoiseger]

Fixed #68, I didn't want to wait. I can't say I've ever written an Ansible module before so I am not entirely sure about the documentation format and the like, nor am I sure how to run the tests, but it works in my testing. Basically I am just editing the schema's return value rather than creating our own resource, which is likely the intended usage anyway.

[francoiseger]

Well, CI really isn't liking me. I don't think the issues with the ansible-test suite are my doing, as those are always the same even for code I didn't touch. The linter warnings I think I fixed? Though I don't understand the warning about the trailing newlines seeing as there is only a single new line at the end of the file which is correct.

[Fuochi]

Sanity is coming from ansible-test sanity command. I have left the default checks there. You can run them directly on your local environment with ansible-test sanity --exclude .github/. I'm thinking of providing a container for development purposes, with the environments already setup. but I'm still far from that.
Integration isn't working because you don't have access to CI secrets from fork. I'll have to check the organization and repo settings to fix this. Sorry but you're the first external contributor here.
Meanwhile, I had a quick look at your PR, I've a couple of questions, let me wrap them up, and I'll jot them down

[Fuochi]

I don't get this, let's say I want to specify different scores to different custom formats, am I able to do that?
However, I like the idea of populating the formats from here. Right now there is probably an issue if you add a custom format but you don't update your quality profile with the new custom format. On the first edit it'll surely fail.

[francoiseger]

Yea, custom formats are now just speicifed as

formats:
  AAC: 1000
  DTS: 2000

Or something like that. If you add a custom format and don't update the quality profile nothing should break though. It'll grab the new schema, apply the modifications, see that the result is the same as you get in Sonarr(when a new custom format is added it's added to each quality profile with a score of zero) and everything will be fine?

[Fuochi]

Hi @francoiseger,
I plan to move the api key from secret to var in order to let contributors run the CI pipeline.
Meanwhile I tested your change locally and it does seem to break the test:

fatal: [testhost]: FAILED! => {
    "changed": false,
    "invocation": {
        "module_args": {
            "cutoff": "WEB 720p",
            "cutoff_format_score": 0,
            "formats": null,
            "min_format_score": 0,
            "name": "Example",
            "quality_groups": [
                {
                    "qualities": [
                        "SDTV"
                    ]
                },
                {
                    "name": "WEB 720p",
                    "qualities": [
                        "WEBRip-720p",
                        "WEBDL-720p"
                    ]
                }
            ],
            "sonarr_api_key": "VALUE_SPECIFIED_IN_NO_LOG_PARAMETER",
            "sonarr_url": "http://localhost:8989",
            "state": "present",
            "upgrade_allowed": true
        }
    },
    "msg": "argument 'cutoff' is of type <class 'str'> and we were unable to convert to int: <class 'str'> cannot be converted to an int"
}

Maybe the test task is just having some issue. Since you already tested this, can you share the task you already tested?

[francoiseger]

It should work and this error doesn't make much sense-is it possible that you are running the initial commit of this PR rather than the most recent? That one was broken in just this way.

[francoiseger]

Well the code is still working for me. It's rebased now, maybe CI will like me this time.

[francoiseger]

That should hopefully be all that's needed to fix this. If not, I'll only really have time to take a more in depth look in two weeks or so.

[RomLecat]

Any chance this could be merged ? The inability of ordering disabled qualities is a pretty big issue.

[Fuochi]

Hi @RomLecat I'm sorry but this has diverged too much, it's not going to be merged. The issue this PR was solving (#68) was about missing disabled qualities, and should be solved since v1.1.1 but neither this PR, nor the fix that was merged, is giving you the ability of ordering disabled qualities.
Why do you need to order the disabled qualities? is there a Sonarr feature based on that?

[RomLecat]

Yes, Sonarr actually evaluates disabled qualities as well. Enabling/disabling qualities only allows Sonarr to grab them.

For instance, if WEBDL-2160p (disabled) is rated above WEBDL-1080p (enabled) and the files on the disk are WEBDL-2160p, Sonarr will not download WEBDL-1080p releases.
If WEBDL-1080p (enabled) is above WEBDL-2160p (disabled, on disk), Sonarr will download WEBDL-1080p release and replace WEBDL-2160p files on disk.

This PR also was more convenient at managing qualities by their name, instead of having to fetch them using info module beforehand.

[Fuochi]

Understood, thanks! I guess I've just never encountered the case where there are episodes in a quality not enabled on the quality profile itself.
As I was writing in my previous comment, I don't think this PR would solve this issue, since in both the current behaviour and this version you'll have to specify only the allowed qualities, while the others are just being added by the code to fill out the whole profile.

Could you please open a new issue about ordering disabled qualities?

About the quality name, it is a nice feature indeed. I just thought that fetching the quality by info module was less error prone. We can think of changing that, but we'll have to make a breaking change for this module.
For now, let's just focus on the issue you just highlighted. But, if you prefer, you can open another issue for this as well, and we can track them separately!

[RomLecat]

Sure, thanks for getting back to me!
I opened #90 for the abiliy to move qualities around and #91 for the "nice to have" name feature.