Add channel field to version and set it on version submission

[jvillalobos]

Part of #3477

This is only adding the channel column to the version entity and setting it with the same value as is_listed from addon when a new version is uploaded. So, Unlisted would be 0 and Listed would be 1. Note that we might need a third channel in the future, so the column should be numerical.

[eviljeff]

what would the third channel be? And is there a requirement to use (those exact) numeric values to represent the channels?

[jvillalobos]

If we decided to support beta versions as one of the channels, then that would be the third one. From discussions we had last week, it looks like won't.

There's no requirement to use those exact numbers. I suggested them because they would match the current value of is_listed.

[eviljeff]

I'm looking into using File.status instead of adding another property we need to check and that would work with a beta channel if it was decided to keep it.

is_listed is defined as a boolean (though obviously commonly interpreted as 0 and 1) so we should be careful of treating channel similarly - if unlisted=0; listed =1; beta=2 then if channel: would match for other non-listed values, such as the beta channel.

[jvillalobos]

I'd like the channel to be handled at the version level, not the file level. Having to drill down to the file to figure anything out about the version status makes things unnecessarily complex, I think. Eventually I'd like to move the status up to the version so files become less important, but that will happen later.

[eviljeff]

Well... all queries already have to operate at the file level because that's where the status is to determine if the file is approved, etc. (the only 'status'-like thing at the version level is deleted). If we're lucky most of the exciting view/update code would trivially work because it already ignores any files not STATUS_PUBLIC. (Mixed listed/unlisted files on a version won't be a thing)

What's the use-case for storing the property on the version? (other than making your weekly sql queries easier 😛 )

[jvillalobos]

I don't want us to overload status flags like we did with add-on and files, where for example Disabled by Mozilla should be an admin flag and the other statuses represent the review status. This kind of thing makes it difficult to manage add-ons because admin actions override the review status of an add-on and they can be difficult to revert (less so without prelim status, but still...). I'd like the review status to be independent from the channel to avoid similar problems.

[eviljeff]

Regardless of whether channel is a version property or defined elsewhere File status is already not really purely just the review status. Being STATUS_PUBLIC means it's currently signed and in a location on the CDN that makes it accessible (both for Listed and Unlisted, for different levels of accessible).

What status would a File on an Unlisted version have?

[jvillalobos]

Unlisted files are either public, which really just means signed, or disabled by an admin. It's not quite a review status, but it's still independent from a version being listed or unlisted.

[eviljeff]

Unlisted files are either public, which really just means signed, or disabled by an admin. It's not quite a review status, but it's still independent from a version being listed or unlisted.

Yes, so an admin action to change a listed version to an unlisted version would necessitate the status on the files being changed to STATUS_PUBLIC, making it the status dependant on the channel. Making channel a version property is doable if you want, sure, but it's not going to fix the overloading.

[jvillalobos]

I can't imagine a case where we want admins changing a version or file from listed to unlisted, or viceversa. It can lead to problems with reviewing, signing, visibility, etc. Being able to only change the review status without accidentally changing the channel is a benefit IMO.

[diox]

I'm looking into using File.status instead of adding another property we need to check and that would work with a beta channel if it was decided to keep it.

I don't think we gain much by re-using File.status. We'd need new statuses for every case - otherwise
you lose the information about the channel as soon as the file goes "public" in its corresponding channel - which complicate the status workflow, reproducing the mess we had with preliminary making things much harder to understand...

Well... all queries already have to operate at the file level because that's where the status is to determine if the file is approved, etc. (the only 'status'-like thing at the version level is deleted). If we're lucky most of the exciting view/update code would trivially work because it already ignores any files not STATUS_PUBLIC. (Mixed listed/unlisted files on a version won't be a thing)

Like with the addition of the Version.channel field, we'd still need to go through all statuses checks anyway to sure things are correct.

If we didn't add new statuses and made both unlisted and beta versions a channel through an additional field, status really becomes the state of the file. It does not matter if it's part of a listed version or not, status indicates what state the file is in. We could even possibly look into making STATUS_PUBLIC less confusing by renaming it something like STATUS_SIGNED.