search

LearnersGuild / game-prototype

Lightweight, minimal implementation of game mechanics for rapid experimentation and prototyping.
0 stars 0 forks source link

Players can submit retro for any project independently of cycle state #137

Closed tannerwelsh closed 7 years ago

tannerwelsh commented 7 years ago

Critical Strategic Goals

Benefits

What are the benefits of this change, and whom do they impact?

Description

Describe the change, and provide any needed context.

Players can /retro #project at any time to submit a retro for any project they have been on (unless they've already submitted a retro for that project).

Logic changes

Players can submit a retro for any of their projects that have been marked complete

With this, there is no need for a cycle to have a REFLECTION state.

UI changes

/retro

/retro will load survey for the player's most recent completed project, if they have only one. Otherwise, will let them know that they have to choose one of their projects (no change from current UI/UX)

/retro

When submitting a retro for a particular project...

If the player has not already submitted a retro for that project: -> open the retro survey for that project

If they player has already submitted a retro for that project: -> show the following screen and error message

[view message:] The survey could not be loaded.

[error message:] Error: survey for project #<project-id> already submitted. To request an amendment to your response data, submit a new issue here: https://github.com/LearnersGuild/los/projects/1
tannerwelsh commented 7 years ago

Logic + UI changes added. Ready for review @LearnersGuild/software

jeffreywescott commented 7 years ago

So is there a hidden implication here that stats will be (re-)calculated after each new retro submission? If so, I'd recommend that we prioritize this after we have a pull-based stats UI (https://github.com/LearnersGuild/game-prototype/issues/109) to avoid confusion.

tannerwelsh commented 7 years ago

You're right, there is a hidden implication @jeffreywescott. Now unhidden: #145

Prioritization is up to you, but frankly I think this can and should be done before #109. The confusion it may cause is less than the benefit we'd get from fewer arbitrary locks on data collection. Also, people are already confused by how stat calculation works :/

jeffreywescott commented 7 years ago

So ... will we still only send one DM w/ stats, or a new DM every time the stats are re-calculated?

shereefb commented 7 years ago

Not sure I understand what you're saying jeffrey. We're talking about retro not review. Stats are calculated one time once all team members have submitted their retros. That's when the DM goes out.

jeffreywescott commented 7 years ago

Good point, @shereefb -- sorry -- I was confused, indeed.

bundacia commented 7 years ago

I like this change. Two pieces of feedback:

1. Keep the Reflection State

I could easily see people accidentally submitting reviews for the current project on Mondays when they intended to submit one for the previous week since the forms will look identical except for the name at the top.

I'm wondering if we should keep the REFLECTION state (at least for a while). And have it work this way:

  1. Before a cycle enters REFLECTION you can't submit reviews for a project in that cycle.
  2. When the cycle is not yet in a REFLECTION state, typing /retro pulls up the review survey for your most recent past project. That way when people type retro on the first day of the cycle the systems acts as they would expect: pulling up the review for the project they just completed, not the one they just started.

We could implement this my adding a completed flag to projects and just setting that flag for all projects in a cycle when the cycle enters REFLECTION. Then we can default the /retro command to act on your "most recent completed project". That logic would continue to work as we decouple projects from cycles more; we could just provide a way for teams to mark their project completed manually. At that point the REFLECTION state could go away altogether. I suppose we could move to this model now, but it just adds work for all teams right now since currently all projects get completed at the same time.

2. What about /review?

Does this change affect how /review works at all? Should it?

tannerwelsh commented 7 years ago

@bundacia thanks for catching that. What if we removed the default retro/review entirely? I.e. we make the #project-id parameter for both commands required, so people have to specify the project?

bundacia commented 7 years ago

@tannerwelsh: I'm not sure that would solve all our problems. We'd still be allowing people to complete a retro for a project that was not complete. Since the surveys would have to be created "on demand" (instead of at /cycle reflect like they are now). You could run into strange cases like this:

  1. Someone accidentally issues a /retro #my-brand-new-project command on the first day of the cycle. Even if they realize their mistake and don't complete the survey, it's still been generated in the system for that project.
  2. Players get added to removed from that team
  3. Now the players the retro questions are asking about are out of sync with the actual players on the project.

So the even though the "just make the project name a required arg" fix would allow us to remove logic around finding the right project, which would simplify the system, we'd still have to add a bit of complexity in other areas to mitigate cases like the one above.

jeffreywescott commented 7 years ago

@bundacia, it seems like (if we remove the REFLECTION state), when a project team changes composition (team members are added or removed), the system would have to delete any retrospectives that were submitted with the previous team composition, because they'd no longer be valid, right? Especially since people cannot change their retro answers once they've submitted them.

jeffreywescott commented 7 years ago

Moving back to Review -- this isn't fully baked.

bundacia commented 7 years ago

@jeffreywescott, yeah we'd have to do something like that, which seems a bit complex. I'd rather avoid jumping through all those hoops if we don't need to.

Reading over my suggestion again, I'm realizing we don't actually need the REFLECTION state at all. We can just change the /cycle reflect command to mark all of the projects complete. It's not really a state, just an event. Maybe we'd rename the command in that case. It would then be trivial in the future to add commands to mark individual projects complete.

jeffreywescott commented 7 years ago

Nice thinking, @bundacia -- so do you think this is now RFI? I'll move it back over if so.

bundacia commented 7 years ago

@jeffreywescott, the description would still need to up updated first I think. The Logic Changes section should read:

#### Logic changes

Players can submit a retro for any of their projects that have been marked complete

With this, there is no need for a cycle to have a `REFLECTION` state.

- Remove `REFLECTION` state from cycle model
- Change `/cycle reflect` command so that it just markes all cycle projects complete.
- Allow players to submit retro surveys for any completed project that they have been on (via the `/retro #project` command)

We also need to decide weather to continue to support /retro without an explicit project name, and update the description if not.

shereefb commented 7 years ago

It seems to me that cycles should no longer haves states, and projects should track their states.

Cycles should be aware of their project lists, but projects should not care or know anything about their cycles.

Agree with @bundacia that cycle commands simply trigger their projects into a different state. So /cycle reflect switches all its projects to REFLECTION state, not COMPLETE. Once all retros and enough reviews are submitted for a project it goes into COMPLETE state and no more retros are allowed there.

We're going to need a REFLECTION state for projects to differentiate between projects that have a complete set of retros/reviews. For example, projects in REFLECTION state can nag/remind team members to get their retros in.

/retro command should not infer the project, and project id should be passed to it.

bundacia commented 7 years ago

We're going to need a REFLECTION state for projects to differentiate between projects that have a complete set of retros/reviews. For example, projects in REFLECTION state can nag/remind team members to get their retros in.

I could definitely see us needing this in the future, but I'd rather create this extra reflection state for projects once we have a concrete use case for it. Not sure we need it now. Either way that's more of an implementation decision. Either way would support the use case for this issue, allowing players to submit retros for any project independent of cycle state.

jeffreywescott commented 7 years ago

Okay, thanks, all. I've updated it as @bundacia suggested. We'll definitely want to do more work around decoupling cycle state from project state in the future, but that's definitely beyond the scope of this particular issue. Moving to RFI, and then eventually implementation.

jeffreywescott commented 7 years ago

Issue moved to LearnersGuild/game #629 via ZenHub