search

CoderDojo / community-platform

Zen, the CoderDojo Community Platform!
https://zen.coderdojo.com
MIT License
121 stars 55 forks source link

Capacity limit not adhered to on auto approval event #778

Closed tangentfairy-zz closed 8 years ago

tangentfairy-zz commented 8 years ago

This dojo has a session with mentor tickets. There are 2 allocated but three have managed to sign up.

Blurred out details of names.

screen shot 2015-11-05 at 09 45 37

SemanticallyNull commented 8 years ago

Currently looking into this and ran into a question. If a Dojo is not using auto-approval then you can have applications that have not yet been approved but may be approved in the future. What behaviour do we want if there are spaced filled by unapproved applications? Options which I've considered:

  1. Do nothing, allow a user to book one or more tickets and expect the approver to deal with the issue.
  2. Allow a user to book tickets but show a warning saying that the tickets may not be approved and expect the approver to deal with emailing telling them the tickets aren't going to be approved due to capacity.
  3. Do not allow the user to book tickets, freeing them up only when the approver deletes unaccepted applications.
tangentfairy-zz commented 8 years ago

I would be in favour of option 3, what do you think?

On Sunday, November 8, 2015, Ben Chapman notifications@github.com wrote:

Currently looking into this and ran into a question. If a Dojo is not using auto-approval then you can have applications that have not yet been approved but may be approved in the future. What behaviour do we want if there are spaced filled by unapproved applications? Options which I've considered:

  1. Do nothing, allow a user to book one or more tickets and expect the approver to deal with the issue.
  2. Allow a user to book tickets but show a warning saying that the tickets may not be approved and expect the approver to deal with emailing telling them the tickets aren't going to be approved due to capacity.
  3. Do not allow the user to book tickets, freeing them up only when the approver deletes unaccepted applications.

— Reply to this email directly or view it on GitHub https://github.com/CoderDojo/community-platform/issues/778#issuecomment-154832029 .

Ursula Clarke Everett Technical Lead @ CoderDojo Foundation S: tangentfairy | T: @CoderDojo | @tangentfairy

Volunteer at your local Dojo and inspire the next generation of digital creators!

Hello World Foundation operating as the CoderDojo Foundation is a company limited by guarantee not having a share capital with charitable status, CHY

  1. Registered in Dublin, Ireland, No 524255. Registered Office: Dogpatch Labs, Unit 1, CHQ Building, Custom House Quay, Dublin 1, Ireland.
SemanticallyNull commented 8 years ago

Yeah, probably the best option for now.

SemanticallyNull commented 8 years ago

@tangentfairy The two pull requests above should sort out this issue. The events service must be deployed first, as the UI changes rely on it.

tangentfairy-zz commented 8 years ago

Great thanks, will review! :)

tangentfairy-zz commented 8 years ago

Applying for a ticket works but I'm having an issue editing an existing event when logged in as a champion.

SemanticallyNull commented 8 years ago

OK, I see the issue... it automatically tries to persist calculated values to the database. That causes the request to completely topple the server.

I've put in a solution that solves the problem but feels incorrect... do you think there's a better way of structuring this solution?

Maybe I should be splitting out this check into a separate call, but since it's a separate call per ticket type it seems unnecessarily expensive.

SemanticallyNull commented 8 years ago

Looking into it further I think it might be worth filtering the object to be persisted to the database. This way we know it only contains keys which can be saved to the database, rather than trying to filter out anything that gets added later.

Something like this would probably be a bit nicer:

var ticket = _.pick(args.ticket, function(key) {
  return ['id', 'sessionId', 'name', 'type', 'quantity', 'deleted', 'invites'].indexOf(key) !== -1;
});
if (!ticket) return callback(null, {error: 'args.ticket is empty'});
ticketEntity.save$(ticket, callback);

Downside is this has to be maintained with the schema, but the upside being the app doesn't fall over when someone tries to fill in a field which isn't on the DB.

tangentfairy-zz commented 8 years ago

That sounds good @BenChapman :)

SemanticallyNull commented 8 years ago

Final commit should've fixed that :) Ready to go @tangentfairy

tangentfairy-zz commented 8 years ago

Tested it and it looks good now, will look at deploying asap

tangentfairy-zz commented 8 years ago

Jenkins failed on the cp-events-service

> cp-events-service@0.0.1 lint /var/lib/jenkins/jobs/cp-events-service-codedeploy/workspace
> semistandard *.js lib/*.js config/config.js

semistandard: Use Semicolons For All! (https://github.com/Flet/semistandard) 
  /var/lib/jenkins/jobs/cp-events-service-codedeploy/workspace/lib/bulk-apply-applications.js:61:36: Missing space before function parentheses.
  /var/lib/jenkins/jobs/cp-events-service-codedeploy/workspace/lib/bulk-apply-applications.js:62:49: Missing space before function parentheses.
  /var/lib/jenkins/jobs/cp-events-service-codedeploy/workspace/lib/bulk-apply-applications.js:64:29: Expected '===' and instead saw '=='.
  /var/lib/jenkins/jobs/cp-events-service-codedeploy/workspace/lib/bulk-apply-applications.js:66:9: Keyword "if" must be followed by whitespace.
  /var/lib/jenkins/jobs/cp-events-service-codedeploy/workspace/lib/save-ticket.js:10:44: Missing space before function parentheses.
  /var/lib/jenkins/jobs/cp-events-service-codedeploy/workspace/lib/search-tickets.js:24:111: Missing space before function parentheses.
  /var/lib/jenkins/jobs/cp-events-service-codedeploy/workspace/lib/search-tickets.js:27:16: A space is required after ','.
  /var/lib/jenkins/jobs/cp-events-service-codedeploy/workspace/lib/search-tickets.js:34:109: Strings must use singlequote.
  /var/lib/jenkins/jobs/cp-events-service-codedeploy/workspace/lib/search-tickets.js:34:131: Missing space before function parentheses.
  /var/lib/jenkins/jobs/cp-events-service-codedeploy/workspace/lib/search-tickets.js:37:16: A space is required after ','.
tangentfairy-zz commented 8 years ago

Edit: One major issue with this now

tangentfairy-zz commented 8 years ago

Fixed, deployed and confirmed working now.

tangentfairy-zz commented 8 years ago

Happening again on Docklands dojo:

screen shot 2016-02-09 at 09 19 39

There should be 35 youth tickets booked but the event has 38, and scratch is sold out but tickets are still bookable:

screen shot 2016-02-09 at 09 20 33

tangentfairy-zz commented 8 years ago

I fixed this in https://github.com/CoderDojo/cp-events-service/commit/78badb530e47f2f2af42496022d12a21a70e0420 and just deployed it. CC @Giustina