Skip to content

Ability to create a script with lockable stages#9703

Merged
Bjvanminnen merged 6 commits into
stagingfrom
dsl_lockable_stages
Jul 26, 2016
Merged

Ability to create a script with lockable stages#9703
Bjvanminnen merged 6 commits into
stagingfrom
dsl_lockable_stages

Conversation

@Bjvanminnen

@Bjvanminnen Bjvanminnen commented Jul 26, 2016

Copy link
Copy Markdown
Contributor

This makes it so that we can set stages as lockable using our script editor on levelbuilder (i.e. /s/script_name/edit). This was done by making it so that the last param for stages can either be a string (in which case it's just a flex_category - i.e. existing behavior) or an options hash that can include flex_category and/or lockable.

Not sure if it might be better to convert all existing usages of flex_category to use the options hash, and then get rid of the special casing (not sure how many existing instances there are or how much retraining of LBs this would require).

Right now, we do send back to the client whether the stage is lockable or not, but don't actually do anything with it.

Comment thread dashboard/test/dsl/dsl_test.rb Outdated
level 'Level 1'
stage 'Stage2'
level 'Level 2'
"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: prefer heredoc.

@joshlory

Copy link
Copy Markdown
Contributor

LGTM

Comment thread dashboard/app/dsl/script_dsl.rb Outdated
string :wrapup_video

def stage(name, flex = nil)
# Can either provide a flex_category as an optional param, or an options hash

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should lock to always using an options hash, and update existing scripts.

@Bjvanminnen

Copy link
Copy Markdown
Contributor Author

I went ahead and removed the ability to add flex_category without using a property hash, and then updated all existing usages. I made one script use the old approach and validated that things broke when I ran rake seed:all (giving me confidence that I caught all usages after fixing the one that I intentionally broke).

I'll also wait on circle-ci which presumably runs seed:all as well.

There will be some opportunity for LBs to introduce problems if they add a flex category between now and when this makes it to the LB machine, but I don't think we need to be too worried about that.

@Bjvanminnen Bjvanminnen merged commit 978420f into staging Jul 26, 2016
@Bjvanminnen Bjvanminnen deleted the dsl_lockable_stages branch July 26, 2016 22:08
@Bjvanminnen

Copy link
Copy Markdown
Contributor Author

If anyone does run into seeding problems in the next couple days related to this, the fix is to make sure new usages use the options hash for flex category (i.e. instead of stage 'stage name', 'content' do stage 'stage name', flex_category: 'content'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants