Show week DB migration

[tuz666]

Why make changes? State your reason

This change will let our radio to have multiple (4) schedules for different weeks.
Two biweekly shows can run in the same slot parallelly alternating between odd and even weeks.

What has changed? Describe your work

The data type of show's week column was changed from integer to integer[].
The current state of the DB was migrated in a way which provides smooth transition both for the frontend and mobil app.

Until now there were 2 usage of the Show's week column: 0 for pop-up, temporary shows and 1 for regular, active shows.
0 values were replaced to [4] and 1 to [0, 1, 2, 3]

The endpoints are still going to use week=1 filter as default, the only difference is that the filter will be executed against an array of integers.

How to review? Add scenarios or information on the output

By reverting the relevant empty_dump.sql changes (L156,L345) it is pretty easy to validate the upgrade and downgrade scripts.

Notes. Share the github issue link, source link, or anything else

[pvj]

This is nice and simple implementation, the migration is great as well. Just to play the devil's advocate: What happens if someone edits show to f.e.

a, week 8? This should be the same as week 4
or
b, -1? Stays -1, I wonder what happens?
or
c, 0? This should be the same as week 4 (or 8)

d, I guess if the order is random that doesn't matter, right?

b, -- is week number can only be positive, this is solved
a, c, -- these should be resolved correctly to their particular week indexes we just need to make sure that it is not displayed / sent through API / used in code because in all these places it leads to confusion. Ie. the correct way to work with week lists when displaying / sending in response / computing in code is to use the calculated remainder. Api should send result of [(1%4) (2%4)], [(3%4)], arcsi should only display (0%4)+1 or use ABC, and code should never use purely values of the week list.

We should still probably save the week as-is (0, 8, 7, 22) cause if we ever need to move from 4 weeks to more (any?) then those come in hand.

Do you think we are OK in all these cases? Please confirm then let's go.

The quality of life fixes and changes around the infra and env are good addition, thanks for the work with it!

Some general thoughts but this is a separate issue:
Stuff like Show.query.filter(Show.active == True) , Show.week.any(abcd_week) on the backend and week_number.strip() for week_number in show_metadata["week"].split(",") or

{% set tags = [] %}
      {% for tag in item.tags %}

on front or templates, etc. so things which gets repeated a lot make me think, after the media we need to extract these into a class method or helper function on a branch.

[pvj]

Makes sense good finding!

[tuz666]

Thanks for the comments, let me try to answer them!

This is nice and simple implementation, the migration is great as well. Just to play the devil's advocate: What happens if someone edits show to f.e.
a, week 8? This should be the same as week 4
b, -1? Stays -1, I wonder what happens?
c, 0? This should be the same as week 4 (or 8)
d, I guess if the order is random that doesn't matter, right?

Meanwhile thinking about the implementation both from backend and frontend side, I thought only 0,1,2,3 and their permutations as valid values.
Maybe it would be wise to add a limitation/validation to only enable these 4 values.
Both frontend and mobile app are going to use one of these 4 numbers, this would have prevent any serious failures.
Ofc if one of the show hosts changes their show's week to -1 that show will be missing from the schedule page.

b, -- is week number can only be positive, this is solved a, c, -- these should be resolved correctly to their particular week indexes we just need to make sure that it is not displayed / sent through API / used in code because in all these places it leads to confusion. Ie. the correct way to work with week lists when displaying / sending in response / computing in code is to use the calculated remainder. Api should send result of [(1%4) (2%4)], [(3%4)], arcsi should only display (0%4)+1 or use ABC, and code should never use purely values of the week list.

We should still probably save the week as-is (0, 8, 7, 22) cause if we ever need to move from 4 weeks to more (any?) then those come in hand.

You highlighted something here what I totally forgot.
The calculation of the remainder supposed to be on the backend side.
The frontend and app shouldn't bother with these things, we can expect that they will query the number of the current and the upcoming three weeks.

Do you think we are OK in all these cases? Please confirm then let's go.

I will add the % 4 calculation for sure and have a look on the validation of the entries of the week list.

I was also thinking about introducing a separate edit form for admins and hosts.
The scheduling (frequency, week, day, start, end) and Azura (playlist_name) related fields could be excluded from the later one, but I don't want to extend the scope of this change with that one.

The quality of life fixes and changes around the infra and env are good addition, thanks for the work with it!

Some general thoughts but this is a separate issue: Stuff like Show.query.filter(Show.active == True) , Show.week.any(abcd_week) on the backend and week_number.strip() for week_number in show_metadata["week"].split(",") or

{% set tags = [] %}
      {% for tag in item.tags %}

on front or templates, etc. so things which gets repeated a lot make me think, after the media we need to extract these into a class method or helper function on a branch.

Makes sense I will have a look on these as well.