From d2b63d0935e32e113fcae23eb62d15f320f66bcb Mon Sep 17 00:00:00 2001
From: chillaranand <anand21nanda@gmail.com>
Date: Tue, 26 Apr 2022 11:21:14 +0530
Subject: [PATCH 1/6] feat: Added force flag to install-app command

---
 frappe/commands/site.py          |  5 +++--
 frappe/core/doctype/file/file.py |  4 ++--
 frappe/installer.py              | 12 ++++++------
 frappe/modules/import_file.py    |  2 --
 frappe/utils/install.py          |  1 +
 5 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/frappe/commands/site.py b/frappe/commands/site.py
index fa9ab4be59..71a6ccb544 100644
--- a/frappe/commands/site.py
+++ b/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))
diff --git a/frappe/core/doctype/file/file.py b/frappe/core/doctype/file/file.py
index 3547a03832..8ca0b9ea10 100755
--- a/frappe/core/doctype/file/file.py
+++ b/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()
diff --git a/frappe/installer.py b/frappe/installer.py
index 634d6287f8..e71c08b859 100644
--- a/frappe/installer.py
+++ b/frappe/installer.py
@@ -226,7 +226,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 +243,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,7 +251,7 @@ 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:
+	if not force and name in installed_apps:
 		frappe.msgprint(frappe._("App {0} already installed").format(name))
 		return
 
@@ -266,7 +266,7 @@ 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)
 
@@ -573,13 +573,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():
diff --git a/frappe/modules/import_file.py b/frappe/modules/import_file.py
index 00483bf6a5..d39f98f966 100644
--- a/frappe/modules/import_file.py
+++ b/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,
diff --git a/frappe/utils/install.py b/frappe/utils/install.py
index 08a399e5fe..4342fb037f 100644
--- a/frappe/utils/install.py
+++ b/frappe/utils/install.py
@@ -46,6 +46,7 @@ def after_install():
 
 	if not frappe.conf.skip_setup_wizard:
 		frappe.db.set_default("desktop:home_page", "setup-wizard")
+		frappe.db.set_value("System Settings", "System Settings", "setup_complete", 0)
 
 	# clear test log
 	with open(frappe.get_site_path(".test_log"), "w") as f:

From fc6093515db683056d6bfbc98acd1aa1e71c3e99 Mon Sep 17 00:00:00 2001
From: phot0n <ritwikpuri5678@gmail.com>
Date: Mon, 9 May 2022 00:40:06 +0530
Subject: [PATCH 2/6] chore: add comments and add force flag to more places

---
 frappe/installer.py | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/frappe/installer.py b/frappe/installer.py
index e71c08b859..7d612469aa 100644
--- a/frappe/installer.py
+++ b/frappe/installer.py
@@ -80,7 +80,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)
 
@@ -252,8 +258,7 @@ def install_app(name, verbose=False, set_as_patched=True, force=False):
 		raise Exception("App not in apps.txt")
 
 	if not force and name in installed_apps:
-		frappe.msgprint(frappe._("App {0} already installed").format(name))
-		return
+		raise Exception("App {0} already installed".format(name))
 
 	print("\nInstalling {0}...".format(name))
 
@@ -268,7 +273,7 @@ def install_app(name, verbose=False, set_as_patched=True, force=False):
 	if name != "frappe":
 		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)
 

From ef99b57a08a5a42a18230fe8bc7838c2d2f5a590 Mon Sep 17 00:00:00 2001
From: phot0n <ritwikpuri5678@gmail.com>
Date: Mon, 9 May 2022 01:06:18 +0530
Subject: [PATCH 3/6] chore: use secho instead of raising exception if app is
 already installed

* chore: singular click import
---
 frappe/installer.py | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/frappe/installer.py b/frappe/installer.py
index 7d612469aa..5cd46e618d 100644
--- a/frappe/installer.py
+++ b/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
@@ -258,7 +260,8 @@ def install_app(name, verbose=False, set_as_patched=True, force=False):
 		raise Exception("App not in apps.txt")
 
 	if not force and name in installed_apps:
-		raise Exception("App {0} already installed".format(name))
+		click.secho(f"App {name} already installed", fg="yellow")
+		return
 
 	print("\nInstalling {0}...".format(name))
 
@@ -320,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)
@@ -757,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",
@@ -803,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):

From 5b4fb3f2cfeb6ed7844d566b8d83a551e96deb4a Mon Sep 17 00:00:00 2001
From: phot0n <ritwikpuri5678@gmail.com>
Date: Tue, 10 May 2022 16:29:47 +0530
Subject: [PATCH 4/6] fix: don't add navbar items if theyre already present

* fix: don't set installed_apps global value in after_install hook
---
 frappe/utils/install.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/frappe/utils/install.py b/frappe/utils/install.py
index 4342fb037f..1a0c83ffe8 100644
--- a/frappe/utils/install.py
+++ b/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()
 
@@ -242,6 +239,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",

From c89a66a694b73d2d1fc4e746e0e36f5dc26a2812 Mon Sep 17 00:00:00 2001
From: phot0n <ritwikpuri5678@gmail.com>
Date: Tue, 10 May 2022 23:17:16 +0530
Subject: [PATCH 5/6] fix: only set home_page in after_install, if the key
 doesn't have value

---
 frappe/utils/install.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/frappe/utils/install.py b/frappe/utils/install.py
index 1a0c83ffe8..2ce067a018 100644
--- a/frappe/utils/install.py
+++ b/frappe/utils/install.py
@@ -42,8 +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")
-		frappe.db.set_value("System Settings", "System Settings", "setup_complete", 0)
+		# 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:

From d28eb94281b201b51251faa79ef95a79ddaabc88 Mon Sep 17 00:00:00 2001
From: phot0n <ritwikpuri5678@gmail.com>
Date: Wed, 11 May 2022 01:15:20 +0530
Subject: [PATCH 6/6] test: test for force install-app

Note: this is a very minimal test, as frappe_docs doesn;t really have any doctypes
---
 frappe/tests/test_commands.py | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/frappe/tests/test_commands.py b/frappe/tests/test_commands.py
index 67de6c6e41..18f18bc704 100644
--- a/frappe/tests/test_commands.py
+++ b/frappe/tests/test_commands.py
@@ -460,6 +460,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 = {