Write script_id when creating new ChannelTokens #39855
Conversation
maureensturgeon
force-pushed
the
maureen/LP-1838-channel-tokens-script-id
branch
from
17d6417
to
12e147e
Compare
maureensturgeon
force-pushed
the
maureen/LP-1838-channel-tokens-script-id
branch
2 times, most recently
from
a6de371
to
c220548
Compare
maureensturgeon
force-pushed
the
maureen/LP-1842-channel-tokens-write-script-id
branch
from
c63ea2e
to
9a34d9a
Compare
|
Do we currently have any levels that are shared between scripts (i.e. do we currently have a "live" instance of this bug)? |
| @@ -178,7 +179,7 @@ def app_options | |||
|
|
|||
| if (@level.channel_backed? && params[:action] != 'edit_blocks') || @level.is_a?(Javalab) | |||
| view_options( | |||
| channel: get_channel_for(@level, @user), | |||
| channel: get_channel_for(@level, @script&.id, @user), | |||
There was a problem hiding this comment.
from the Jira epic, it looks like you are planning to add a not-null constraint on script_id in the future. This suggests to me that you'll need to come up with a plan for what to do on lines like this for when @script is nil. cases which come to mind include (1) standalone projects, and (2) when viewing a level by id in levelbuilder mode. I'm not sure we have tests covering the latter. I'll see if I can add one.
There was a problem hiding this comment.
So the ticket with the non-null constraint was resolved with status "won't do" https://codedotorg.atlassian.net/browse/LP-1846, for reason number 2 that you mentioned which is that in levelbuilder we'll be able to view a level that's channel-backed outside of the context of a script so script_id will be nil.
There was a problem hiding this comment.
Great! No concerns with instances of @script&.id, then 👍
Yes, check out the comments in this ticket with an example https://codedotorg.atlassian.net/browse/LP-1752 |
| assert_equal expected_channel_token.id, channel_token.id | ||
| end | ||
|
|
||
| test 'find_or_create_channel_token will create a new channel token with script_id if no channel token exists' do |
There was a problem hiding this comment.
Note for future PR: If we decide that passing in nil as the script_id is a valid scenario, let's add a test for it.
maureensturgeon
deleted the
maureen/LP-1842-channel-tokens-write-script-id
branch
Updates
find_or_create_channel_tokento take ascript_idwhich is used only when creating a new channel_token record. All methods callingfind_or_create_channel_tokenare updated accordingly. Note that querying for channel_tokens remains unchanged for now until we have backfilled all the records with empty script_ids.Links
Testing story
Added unit tests & tested locally that starting work on a channel-backed level stores the script id and that channel-backed levels that had work done on them previously load as expected.
Follow-up work
Next up is writing script_id backfills for the existing channel_token records. For the full list of tasks see https://codedotorg.atlassian.net/browse/LP-1395.
PR Checklist: