-
-
Notifications
You must be signed in to change notification settings - Fork 222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove load_appcues_template_app #35807
base: master
Are you sure you want to change the base?
Conversation
@task | ||
def load_appcues_template_app(domain, username, app_slug): | ||
from corehq.apps.app_manager.views.apps import load_app_from_slug | ||
load_app_from_slug(domain, username, app_slug) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can delete this function altogether. Its only remaining purpose is to give devs a way to load template apps (see here), and I'm skeptical anyone would miss it. That should lead to deleting a bunch more code, including the JSON files of the template apps themselves.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll ask to see if anyone uses it, but I suspect you're right and it'll be fine to remove.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given there's some talk of potentially using template apps for other purposes later (like the option to start from a template instead of a blank app in app builder), I think I prefer to do that removal in a separate PR so it can be more easily reverted if we decide to use that functionality again (without accidentally reintroducing default template apps to new accounts). #35817
@@ -13,9 +13,6 @@ <h2>{% trans "Creating Account..." %}</h2> | |||
<p class="timeout-message" data-bind="visible: showSecondTimeout"> | |||
<i class="fa fa-check"></i> {% trans "Created new user..." %} | |||
</p> | |||
<p class="timeout-message" data-bind="visible: showFirstTimeout"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing showFirstTimeout
when there's also a showSecondTimeout
and showThirdTimeout
seem confusing. I'd either update the corresponding javascript or update "Created sample applications" to some other text and keep it ("Prepping application builder"?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. I went with "Prepared project space..." as something fairly generic that matches the tenses of "Created" and "Updated" a4686a7
Product Description
Removes the automatic creation of these 3 template apps for new accounts in SaaS environments:

This will be paired with a removal of the Appcues workflow that highlights these template apps:

This should also have the effect of allowing the new account activation email to send more quickly, since it is not waiting on template apps to be built.
Technical Summary
SAAS-16570
Removes
load_appcues_template_app
and its usage when creating a new account. The code to load a template app generally is retained in app_from_template andload_app_from_slug
, which it calls.Safety Assurance
Safety story
This is a straightforward change and has been tested on staging to see that template apps are not created when a new account is registered.
Automated test coverage
None here.
QA Plan
Not planning it.
Rollback instructions
Labels & Review