Merge pull request #16747 from ChillarAnand/atom

feat: Added force flag to install-app command

frappe/commands/site.py

@@ -398,8 +398,9 @@ def _reinstall(
 
 @click.command("install-app")
 @click.argument("apps", nargs=-1)
+@click.option("--force", is_flag=True, default=False)
 @pass_context
-def install_app(context, apps):
+def install_app(context, apps, force=False):
 	"Install a new app to site, supports multiple apps"
 	from frappe.installer import install_app as _install_app
 
@@ -414,7 +415,7 @@ def install_app(context, apps):
 
 		for app in apps:
 			try:
-				_install_app(app, verbose=context.verbose)
+				_install_app(app, verbose=context.verbose, force=force)
 			except frappe.IncompatibleApp as err:
 				err_msg = ":\n{}".format(err) if str(err) else ""
 				print("App {} is Incompatible with Site {}{}".format(app, site, err_msg))

frappe/core/doctype/file/file.py

@@ -608,7 +608,7 @@ def on_doctype_update():
 def make_home_folder():
 	home = frappe.get_doc(
 		{"doctype": "File", "is_folder": 1, "is_home_folder": 1, "file_name": _("Home")}
-	).insert()
+	).insert(ignore_if_duplicate=True)
 
 	frappe.get_doc(
 		{
@@ -618,7 +618,7 @@ def make_home_folder():
 			"is_attachments_folder": 1,
 			"file_name": _("Attachments"),
 		}
-	).insert()
+	).insert(ignore_if_duplicate=True)
 
 
 @frappe.whitelist()

frappe/installer.py

@@ -7,6 +7,8 @@ import sys
 from collections import OrderedDict
 from typing import Dict, List, Tuple
 
+import click
+
 import frappe
 from frappe.defaults import _clear_cache
 from frappe.utils import is_git_url
@@ -80,7 +82,13 @@ def _new_site(
 	)
 
 	for app in apps_to_install:
-		install_app(app, verbose=verbose, set_as_patched=not source_sql)
+		# NOTE: not using force here for 2 reasons:
+		# 	1. It's not really needed here as we've freshly installed a new db
+		# 	2. If someone uses a sql file to do restore and that file already had
+		# 		installed_apps then it might cause problems as that sql file can be of any previous version(s)
+		# 		which might be incompatible with the current version and using force might cause problems.
+		# 		Example: the DocType DocType might not have `migration_hash` column which will cause failure in the restore.
+		install_app(app, verbose=verbose, set_as_patched=not source_sql, force=False)
 
 	os.remove(installing)
 
@@ -226,7 +234,7 @@ def parse_app_name(name: str) -> str:
 	return repo
 
 
-def install_app(name, verbose=False, set_as_patched=True):
+def install_app(name, verbose=False, set_as_patched=True, force=False):
 	from frappe.core.doctype.scheduled_job_type.scheduled_job_type import sync_jobs
 	from frappe.model.sync import sync_for
 	from frappe.modules.utils import sync_customizations
@@ -243,7 +251,7 @@ def install_app(name, verbose=False, set_as_patched=True):
 	if app_hooks.required_apps:
 		for app in app_hooks.required_apps:
 			required_app = parse_app_name(app)
-			install_app(required_app, verbose=verbose)
+			install_app(required_app, verbose=verbose, force=force)
 
 	frappe.flags.in_install = name
 	frappe.clear_cache()
@@ -251,8 +259,8 @@ def install_app(name, verbose=False, set_as_patched=True):
 	if name not in frappe.get_all_apps():
 		raise Exception("App not in apps.txt")
 
-	if name in installed_apps:
-		frappe.msgprint(frappe._("App {0} already installed").format(name))
+	if not force and name in installed_apps:
+		click.secho(f"App {name} already installed", fg="yellow")
 		return
 
 	print("\nInstalling {0}...".format(name))
@@ -266,9 +274,9 @@ def install_app(name, verbose=False, set_as_patched=True):
 			return
 
 	if name != "frappe":
-		add_module_defs(name)
+		add_module_defs(name, ignore_if_duplicate=force)
 
-	sync_for(name, force=True, reset_permissions=True)
+	sync_for(name, force=force, reset_permissions=True)
 
 	add_to_installed_apps(name)
 
@@ -315,7 +323,6 @@ def remove_from_installed_apps(app_name):
 
 def remove_app(app_name, dry_run=False, yes=False, no_backup=False, force=False):
 	"""Remove app and all linked to the app's module with the app from a site."""
-	import click
 
 	site = frappe.local.site
 	app_hooks = frappe.get_hooks(app_name=app_name)
@@ -573,13 +580,13 @@ def make_site_dirs():
 		os.makedirs(path, exist_ok=True)
 
 
-def add_module_defs(app):
+def add_module_defs(app, ignore_if_duplicate=False):
 	modules = frappe.get_module_list(app)
 	for module in modules:
 		d = frappe.new_doc("Module Def")
 		d.app_name = app
 		d.module_name = module
-		d.insert(ignore_permissions=True, ignore_if_duplicate=True)
+		d.insert(ignore_permissions=True, ignore_if_duplicate=ignore_if_duplicate)
 
 
 def remove_missing_apps():
@@ -752,11 +759,9 @@ def partial_restore(sql_file_path, verbose=False):
 	elif frappe.conf.db_type == "postgres":
 		import warnings
 
-		from click import style
-
 		from frappe.database.postgres.setup_db import import_db_from_sql
 
-		warn = style(
+		warn = click.style(
 			"Delete the tables you want to restore manually before attempting"
 			" partial restore operation for PostreSQL databases",
 			fg="yellow",
@@ -798,8 +803,6 @@ def validate_database_sql(path, _raise=True):
 			error_message = "Table `tabDefaultValue` not found in file."
 
 	if error_message:
-		import click
-
 		click.secho(error_message, fg="red")
 
 	if _raise and (missing_table or empty_file):

frappe/modules/import_file.py

@@ -144,7 +144,6 @@ def import_file_by_path(
 
 			import_doc(
 				docdict=doc,
-				force=force,
 				data_import=data_import,
 				pre_process=pre_process,
 				ignore_version=ignore_version,
@@ -203,7 +202,6 @@ def update_modified(original_modified, doc):
 
 def import_doc(
 	docdict,
-	force=False,
 	data_import=False,
 	pre_process=None,
 	ignore_version=None,

frappe/tests/test_commands.py

@@ -440,6 +440,36 @@ class TestCommands(BaseTestCommands):
 		archive_directory = os.path.join(bench_path, f"archived/sites/{site}")
 		self.assertTrue(os.path.exists(archive_directory))
 
+	@skipIf(
+		not (
+			frappe.conf.root_password and frappe.conf.admin_password and frappe.conf.db_type == "mariadb"
+		),
+		"DB Root password and Admin password not set in config",
+	)
+	def test_force_install_app(self):
+		if not os.path.exists(os.path.join(get_bench_path(), f"sites/{TEST_SITE}")):
+			self.execute(
+				f"bench new-site {TEST_SITE} --verbose "
+				f"--admin-password {frappe.conf.admin_password} "
+				f"--mariadb-root-password {frappe.conf.root_password} "
+				f"--db-type {frappe.conf.db_type or 'mariadb'} "
+			)
+
+		app_name = "frappe"
+
+		# set admin password in site_config as when frappe force installs, we don't have the conf
+		self.execute(f"bench --site {TEST_SITE} set-config admin_password {frappe.conf.admin_password}")
+
+		# try installing the frappe_docs app again on test site
+		self.execute(f"bench --site {TEST_SITE} install-app {app_name}")
+		self.assertIn(f"{app_name} already installed", self.stdout)
+		self.assertEqual(self.returncode, 0)
+
+		# force install frappe_docs app on the test site
+		self.execute(f"bench --site {TEST_SITE} install-app {app_name} --force")
+		self.assertIn(f"Installing {app_name}", self.stdout)
+		self.assertEqual(self.returncode, 0)
+
 
 class TestBackups(BaseTestCommands):
 	backup_map = {

frappe/utils/install.py

@@ -18,9 +18,6 @@ def before_install():
 
 
 def after_install():
-	# reset installed apps for re-install
-	frappe.db.set_global("installed_apps", '["frappe"]')
-
 	create_user_type()
 	install_basic_docs()
 
@@ -45,7 +42,10 @@ def after_install():
 	update_password("Administrator", get_admin_password())
 
 	if not frappe.conf.skip_setup_wizard:
-		frappe.db.set_default("desktop:home_page", "setup-wizard")
+		# only set home_page if the value doesn't exist in the db
+		if not frappe.db.get_default("desktop:home_page"):
+			frappe.db.set_default("desktop:home_page", "setup-wizard")
+			frappe.db.set_single_value("System Settings", "setup_complete", 0)
 
 	# clear test log
 	with open(frappe.get_site_path(".test_log"), "w") as f:
@@ -241,6 +241,10 @@ def add_country_and_currency(name, country):
 def add_standard_navbar_items():
 	navbar_settings = frappe.get_single("Navbar Settings")
 
+	# don't add settings/help options if they're already present
+	if navbar_settings.settings_dropdown and navbar_settings.help_dropdown:
+		return
+
 	standard_navbar_items = [
 		{
 			"item_label": "My Profile",