Move recording variables to Advanced Variables section

[rdswift]

Summary

This is a…

• Correction
• Addition
• Restructuring
• Minor / simple change (like a typo)
• Other

Reason for the Change

The recording variables were shown in the Basic Variables section, when they actually require that the recording level relationships option be enabled.

Description of the Change

Move the recording variables to the Advanced Variables section.

Additional Action Required

Update translation files.

[phw]

We need to check this in more detail, even for more tags. The work series variables are also depending on track relationships. We might even have other work related variables that are only available with track relationships.

Furthermore there is also the release relationships variable, I think the docs currently don't consider this. I think we should check which variables and tags this setting affect. But for sure the release and release group series variables.

[phw]

I looked a bit into it, I think with the comments I added this should work.

I think the work series relationships are also only available when "release relationships" are enabled (this enables both recording-level-rels and work-level-rels in the API requests, see https://github.com/metabrainz/picard/blob/master/picard/album.py#L616-L620).

But then there are also the release series and release group series variables. Those are only available when "release relationships" are enabled. So I assume we need a section in advanced variables which lists only variables available with "release relationships" enabled.

[rdswift]

I've created some subsections of the tags and variables pages, and tried to move each tag and variable into the appropriate section. I've tried to base the catagorization on actual test data (test releases are noted in the "Advanced" pages as a comment), but I wasn't able to confirm all of the items. I also updated some of the item descriptions, and reformatted the files to provide consistent formatting (items on single lines, a single space between sentences, etc.)

I would appreciate it if someone could take a quick look and see if there's anything that is incorrect or could be improved. Thanks.

[phw]

Thanks a lot for this update. This is a real improvement already.

And it is also pretty interesting to dive into these details. We never really dissected it like this and this all more worked on a "feeling". This is also the reason it took me some days to actually review this.

I have a couple of comments and suggestions. Please double check these, it's really easy to get things wrong here.

[phw]

The _releasegroup_series* variables require release relationships enabled. Hence I think they should be in advanced variables in a sections for only available with release relationships.

[rdswift]

Actually, they don't require either the "track relationships" or "release relationships" options to be enabled. I just tested using release "59f6dc82-6e05-4d58-8fae-d93c55a250ef" with both options disabled and the _releasegroup_series variables were all populated.

[phw]

Oh, that's interesting. Didn't expect that. I'll check.

[phw]

One thing is that enabling track Level rels automatically implies release Level rels also being used, see https://github.com/metabrainz/picard/blob/master/picard/album.py#L607 . That's maybe something we need to indicate in the UI or such, but technically it makes sense.

But disabling both I would have expected nothing to be returned. Maybe that is some MB API specific thing how RG relationships are being handled?

I'm currently not at my laptop and can't properly check API calls. But I think if it currently behaves that way we can leave it in the documentation as you currently have it.

[rdswift]

Yeah, I saw that in the Picard code. I never thought to update the UI to clarify but that makes sense. I'll enter a ticket (and PR if I can think of a nice way to show it).

I'll hold off merging this until you have a chance to confirm that it doesn't require either option enabled. I'm so confused by all of this that I'm not trusting my own testing any more. BTW, I actually did my tests in Picard, because I found it easier than trying to sort through the API response and follow how it was processed within Picard. 😉

[rdswift]

Can you test again with e.g. https://musicbrainz.org/release-group/53ed88d5-9b8b-44c8-9e18-7324284c3ec9 ? As expected I get the releasegroup_series tags only if at least release relationships are enabled.

Okay, this is just weird. Running Picard v2.10 installed on a Linux Mint system with both release and track relationships disabled, when I load the release from the release group you specified, I still get the _releasegroup_series variables.

Running Picard v2.10 from the latest source code (master branch) on the same Linux Mint system with both release and track relationships disabled, when I load the release from the release group you specified, I don't get the _releasegroup_series variables.

The other thing (totally unrelated) is when I run from source any selected item disappears -- see Track 1 in the listing. I assume the text color is being set to the background color, and I suspect this is a bug with Qt (or Mint) because it does this with other applications running from source.

In any event, to be on the safe side I'll move those variables in the documentation to show that they require the release relationships enabled. If the user gets them without them enabled, that's just a bonus. 😉

[rdswift]

Now when I test again running the installed version, when I load the release from the release group you specified, I don't get the _releasegroup_series* variables. I wonder if it was somehow retrieving the values from a cached response to the query. Do you know off hand if Picard retains the metadata from a previous load of an album (in the same session) when the album is refreshed? If so, that's likely what I'm seeing.

[rdswift]

I just tested again, and it seems that Picard IS retaining metadata from a prior load of an album. To confirm, try the following:

1. Start Picard.
2. Open the option settings and disable both release and track relationships.
3. Load the test album (release:d8dcd3d2-50d0-47df-ab0a-542606ba5912)
4. Confirm _releasegroup_series* variables are not populated.
5. Open the option settings and enable both release and track relationships.
6. Refresh the test album.
7. Confirm _releasegroup_series* variables are populated.
8. Open the option settings and disable both release and track relationships.
9. Refresh the test album.
10. Confirm _releasegroup_series* variables are populated (even though the relationships are disabled)
11. Remove the test album from the right-hand pane.
12. Load the test album again.
13. Confirm _releasegroup_series* variables are populated (even though the relationships are disabled)

I assume that Picard is caching all of the metadata from the most detailed album retrieval, and that items no longer provided in subsequent retrievals are being filled from the cache. Further, Picard is not clearing this cache when the album is removed. That makes sense to avoid making multiple calls to the API, but it totally screwed up my testing. 😜

[phw]

There is a request cache. But it should be different as the URL changes for the different requests. That's really weird, I also can't reproduce.

[rdswift]

Now that is wierd. It is 100% reproducable here, both using the installed version of Picard and when running from source. Unfortunately, I don't have time right now to try and track down the reason. In any event, with those variables moved to the Advanced section, I think we're covered on the documentation front.