Add a configuration option for badges to be free

[kitsuta]

Our at-door kiosk page automatically sets the price on badges, as well as letting people pay through Stripe right on the kiosk.

This is normally great - however, we have an official unofficial policy of making badges free after noon on Sunday. At MAGFest 12, we had to process several refunds for people who went through the kiosk workflow and paid for a badge they shouldn't have.

Probably the best/most flexible solution is to add a datetime configuration option that sets all badge prices to $0 after that date/time. Then we can easily accommodate our business logic without having to hardcode it. It should be considered 'disabled' if it is not set.

[earl7399]

Would that go in a YAML file? Or development.ini?

[earl7399]

I'm thinking this change in development.ini is what we want:

[badge_prices]
one_days_enabled = true
initial_attendee = 50
dealer_badge_price = 30
badges_free = '2016-02-21 12'

[EliAndrewC]

A better place to put it would probably be [dates] since any new config option we put there will automatically be converted to a datetime object on c.

We probably want to automatically mark badges created after this date as c.NEED_NOT_PAY, since in the past we've put checks in the system to prevent a badge from being marked as "paid" with an overridden price of 0.

Word of warning: this change might be tricker than you'd expect, since we ALSO want to change the frontend display so that after the configured time it updates the badge options to say something like "free" instead of listing a price.

If you think this might not be one you want to tackle, I'd be happy to jump on it, just let me know.

[earl7399]

Hey, this says "Add a configuration option", not make it all work. She's
smart enough not to give me that.

And I'll happily move that to dates.

Robert Earl r.a.earl@gmail.com
FAX: 1-440-919-5086

On Thu, Feb 11, 2016 at 5:15 PM, Eli Courtwright notifications@github.com
wrote:

A better place to put it would probably be [dates] since any new config
option we put there will automatically be converted to a datetime object
on c.

We probably want to automatically mark badges created after this date as
c.NEED_NOT_PAY, since in the past we've put checks in the system to
prevent a badge from being marked as "paid" with an overridden price of 0.

Word of warning: this change might be tricker than you'd expect, since we
ALSO want to change the frontend display so that after the configured time
it updates the badge options to say something like "free" instead of
listing a price.

If you think this might not be one you want to tackle, I'd be happy to
jump on it, just let me know.

—
Reply to this email directly or view it on GitHub
#1682 (comment)
.

[earl7399]

And PyCharm doesn't think development.ini belongs here? Did I break something?

[kitsuta]

Nah, Eli's right, I totally underestimated the task. I thought you would
just need to change what the get_badge_price function in config.py
returned.

On Thu, Feb 11, 2016, 17:39 Robert Earl notifications@github.com wrote:

And PyCharm doesn't think development.ini belongs here? Did I break
something?

—
Reply to this email directly or view it on GitHub
#1682 (comment)
.

[binary1230]

FYI, to answer your question: development.ini is indeed the file that uber reads, however, it's generated on each deploy from scratch by puppet.

puppet consumes the various .yaml files to generate the development.ini file by populating the variables in here: https://github.com/magfest/ubersystem-puppet/blob/master/templates/uber-development.ini.erb

If you're going down this road I can provide a little more detail / writeup on how to add options to the .ini, there's one more step beyond this.

[EliAndrewC]

Don't worry about it, I'll take this one on.

[earl7399]

Dom - thanks, but Vicki explained to me exactly how ... um ... bad this is
(my words). Suddenly I understand why some are pushing so hard for a
config DB!

Eli - thanks for picking this up!

Robert Earl r.a.earl@gmail.com
FAX: 1-440-919-5086

On Thu, Feb 11, 2016 at 8:52 PM, Eli Courtwright notifications@github.com
wrote:

Don't worry about it, I'll take this one on.

—
Reply to this email directly or view it on GitHub
#1682 (comment)
.

[binary1230]

As a sidenote, configDB would just swap out the INI layer, the puppet stuff (or something similar to it) is a different layer and it (or something like it) will always have to remain in order to support production servers for multiple magfest events, and staging vs production/etc.

i.e. configDB doesn't actually simplify any of the complexity issues that puppet currently solves, and is a separate layer.

There's is one (unrelated to configDB) simplification we're hoping to do for puppet that will cut out about two steps and layers for adding new simple config options, it's this: magfest-archive/ubersystem-puppet#28

I just want to make sure that people know just because the system is solving a really complex problem and has a few moving parts doesn't mean it's bad. It just means that the problem it's solving is really hard. I am happy to discuss alternative options that actually solve the multitude of problems which puppet is handling currently.

[binary1230]

(That config stuff is a separate discussion btw from replacing a lot of what puppet is doing with Docker, which I agree that is a direction we should go in)

[earl7399]

It's clear that I don't have a solid grasp on all of this. That's a bad
sign, because I'm not always as dumb as I look. Not being able to add a
simple configuration option in a single place is IMO a bad thing. I
honestly don't understand why the one this issue concerns would not in fact
be added to a single configDB and made available to the code.

That said, the rest of this discussion is for another time (and certainly
not in this issue's comments).

Robert Earl r.a.earl@gmail.com
FAX: 1-440-919-5086

On Fri, Feb 12, 2016 at 7:33 AM, Dominic Cerquetti <notifications@github.com

wrote:

(That config stuff is a separate discussion btw from replacing a lot of
what puppet is doing with Docker, which I agree that is a direction we
should go in)

—
Reply to this email directly or view it on GitHub
#1682 (comment)
.

[EliAndrewC]

Not being able to add a simple configuration option in a single place is IMO a bad thing.

100% agree. But the thing is, it already IS possible to do that. If you need to add a new config option, "all" you have to do is add it to https://github.com/magfest/ubersystem/blob/master/uber/configspec.ini with a default value and it's immediately usable in the code, because it'll automatically go into the global c object.

So why is it so frustrating in practice? The biggest weakness right now is that if you want to add a new config option where MAGFest is going to use a different value than the default one in configspec.ini, you have to make a change in THREE different repos. Not only that, one of the changes is going into a puppet file and writing puppet code - even if that's "easy" and you can just guess what to do based on what we're doing elsewhere, that's awful.

That's what magfest-archive/ubersystem-puppet#28 will fix, if we ever do it. Adding a new config option will be as simple as:

• add an option to configspec.ini as we do now
• if we want to deploy to MAGFest with a different value than the default, add a line to the production-config repo; the name of the config option will be exactly the same as the key name you add to the yaml file there, so there should be no ambiguity or confusion

As I've said, I'm not against ConfigDB, and it definitely has some advantages. I just wanted to point out that if we'd had magfest-archive/ubersystem-puppet#28 all along, I don't think people would find it painful to add new config options today. But it IS painful, and you're right to complain about it.