Page MenuHomePhorge

Signup: Hide plan selection step if there's only one plan
ClosedPublic

Authored by machniak on Tue, Mar 4, 3:35 PM.

Details

Reviewers
mollekopf
Group Reviewers
Restricted Project
Commits
rK43275900b87e: Signup: Hide plan selection step if there's only one plan
Summary
  • Skip plan selection if there is only one active plan
  • Allow signup with a public domain to a group account plan
  • Prefer 'group' over 'individual' plan when the plan is not defined on signup
  • Improvements + tests
Test Plan

./phpunit

Diff Detail

Repository
rK kolab
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

machniak created this revision.
  • Cleanup and lint error fix
  • Introduce DEFAULT_PLAN config option
mollekopf added inline comments.
src/config/app.php
229 ↗(On Diff #14696)

Shouldn't the default be individual according to the code change?

src/config/app.php
229 ↗(On Diff #14696)

Because we also change how signup works and that we allow group account signup with public domain now, I think that 'group' is a better default.

Using plan titles anywhere is not a great solution anyway. Maybe using additional "is_default" flag in plans table would be a better solution. Or maybe we should just get any active plan, and not prefer any specific title. What do you think?

As far as I see the default is used in two cases: 1) with signup invitations, 2) when user somehow provided invalid plan.

src/config/app.php
229 ↗(On Diff #14696)

Having an is_default flag quickly becomes problematic because you can set multiple but you should only have one.

Instead of going with a default perhaps the solution would be to skip the step if there is only one active plan, and otherwise don't skip? So there wouldn't really be a default, it's just you don't have to choose if there's only one option.

In case of an invalid plan we should throw an error and not use a default I think.

So I think the logic could just be:

  • If a plan is specified use that, or bail out if it doesn't exist
  • If no plan is specified, check if we only have a single plan and use that, otherwise bail out.
  • Dashboard elements visibility for a group account without a custom domain
  • Get rid of default plan
  • CS fix
machniak marked an inline comment as done.
  • Cleanup
This revision is now accepted and ready to land.Mon, Mar 10, 8:51 AM