From 2056cc2db996333c91127bb577304b0207ba710b Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Thu, 28 Nov 2024 14:29:02 +0200 Subject: [PATCH 1/9] Report app dependency removal --- corehq/apps/app_manager/models.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/corehq/apps/app_manager/models.py b/corehq/apps/app_manager/models.py index 0f856798df15..3d2dc752a34d 100644 --- a/corehq/apps/app_manager/models.py +++ b/corehq/apps/app_manager/models.py @@ -181,6 +181,7 @@ from corehq.blobs.mixin import CODES, BlobMixin from corehq.const import USER_DATE_FORMAT, USER_TIME_FORMAT from corehq.util import bitly, view_utils +from corehq.util.metrics import metrics_counter from corehq.util.quickcache import quickcache from corehq.util.timer import TimingContext, time_method from corehq.util.timezones.conversions import ServerTime @@ -4467,8 +4468,22 @@ def make_build(self, comment=None, user_id=None): assert copy._id prune_auto_generated_builds.delay(self.domain, self._id) + self.check_build_dependencies(new_build=copy) + return copy + def check_build_dependencies(self, new_build): + """ + This method reports whether the app dependencies have been removed. + """ + + def has_dependencies(build): + return bool(build['profile']['features']['dependencies']) + + latest_build = get_latest_build_doc(self.domain, self.id) + if has_dependencies(latest_build) and not has_dependencies(new_build): + metrics_counter('commcare.app_build.dependencies_removed') + def convert_app_to_build(self, copy_of, user_id, comment=None): self.copy_of = copy_of built_on = datetime.datetime.utcnow() From 717907716934b4964267d6f0f2bf6f270e1cacf5 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Thu, 28 Nov 2024 16:37:33 +0200 Subject: [PATCH 2/9] Patch up method --- corehq/apps/app_manager/models.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/corehq/apps/app_manager/models.py b/corehq/apps/app_manager/models.py index 3d2dc752a34d..bdfa56b3bb52 100644 --- a/corehq/apps/app_manager/models.py +++ b/corehq/apps/app_manager/models.py @@ -4474,13 +4474,18 @@ def make_build(self, comment=None, user_id=None): def check_build_dependencies(self, new_build): """ - This method reports whether the app dependencies have been removed. + Reports whether the app dependencies have been removed. """ def has_dependencies(build): - return bool(build['profile']['features']['dependencies']) + return bool( + build.get('profile', {}).get('features', {}).get('dependencies') + ) latest_build = get_latest_build_doc(self.domain, self.id) + if not latest_build: + return + if has_dependencies(latest_build) and not has_dependencies(new_build): metrics_counter('commcare.app_build.dependencies_removed') From a94b241721aacd27738e245fc9181146a6a12621 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Fri, 29 Nov 2024 11:34:46 +0200 Subject: [PATCH 3/9] Consider that build can be Application instance --- corehq/apps/app_manager/models.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/corehq/apps/app_manager/models.py b/corehq/apps/app_manager/models.py index bdfa56b3bb52..6371f9b0ac7a 100644 --- a/corehq/apps/app_manager/models.py +++ b/corehq/apps/app_manager/models.py @@ -4478,8 +4478,13 @@ def check_build_dependencies(self, new_build): """ def has_dependencies(build): + if isinstance(build, Application): + profile = build.profile + else: + profile = build.get('profile', {}) + return bool( - build.get('profile', {}).get('features', {}).get('dependencies') + profile.get('features', {}).get('dependencies') ) latest_build = get_latest_build_doc(self.domain, self.id) From c2ef0d0572be34cf0e3418e977281296dc9ae455 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Wed, 4 Dec 2024 09:37:03 +0200 Subject: [PATCH 4/9] Rename var --- corehq/apps/app_manager/models.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/corehq/apps/app_manager/models.py b/corehq/apps/app_manager/models.py index 6371f9b0ac7a..a3db45fee1d5 100644 --- a/corehq/apps/app_manager/models.py +++ b/corehq/apps/app_manager/models.py @@ -4487,11 +4487,11 @@ def has_dependencies(build): profile.get('features', {}).get('dependencies') ) - latest_build = get_latest_build_doc(self.domain, self.id) - if not latest_build: + last_build = get_latest_build_doc(self.domain, self.id) + if not last_build: return - if has_dependencies(latest_build) and not has_dependencies(new_build): + if has_dependencies(last_build) and not has_dependencies(new_build): metrics_counter('commcare.app_build.dependencies_removed') def convert_app_to_build(self, copy_of, user_id, comment=None): From af3a6a13220d84ba971eef0e59e1b1364f65cad7 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Wed, 4 Dec 2024 09:51:57 +0200 Subject: [PATCH 5/9] Send metric for app dependecies feature added to build --- corehq/apps/app_manager/models.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/corehq/apps/app_manager/models.py b/corehq/apps/app_manager/models.py index a3db45fee1d5..43aba39b6734 100644 --- a/corehq/apps/app_manager/models.py +++ b/corehq/apps/app_manager/models.py @@ -4487,11 +4487,14 @@ def has_dependencies(build): profile.get('features', {}).get('dependencies') ) + new_build_has_dependencies = has_dependencies(new_build) + last_build = get_latest_build_doc(self.domain, self.id) - if not last_build: - return + last_build_has_dependencies = has_dependencies(last_build) if last_build else False - if has_dependencies(last_build) and not has_dependencies(new_build): + if not last_build_has_dependencies and new_build_has_dependencies: + metrics_counter('commcare.app_build.dependencies_added') + elif last_build_has_dependencies and not new_build_has_dependencies: metrics_counter('commcare.app_build.dependencies_removed') def convert_app_to_build(self, copy_of, user_id, comment=None): From 08b1425cb8e8c3919874bbcc836e43d64eaa7669 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Thu, 5 Dec 2024 07:55:11 +0200 Subject: [PATCH 6/9] Update docstring --- corehq/apps/app_manager/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/corehq/apps/app_manager/models.py b/corehq/apps/app_manager/models.py index 43aba39b6734..fd61230c6735 100644 --- a/corehq/apps/app_manager/models.py +++ b/corehq/apps/app_manager/models.py @@ -4474,7 +4474,7 @@ def make_build(self, comment=None, user_id=None): def check_build_dependencies(self, new_build): """ - Reports whether the app dependencies have been removed. + Reports whether the app dependencies have been added or removed. """ def has_dependencies(build): From 1823cbdc9038182a69e0154cad8695db895c0b9d Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Thu, 5 Dec 2024 08:06:56 +0200 Subject: [PATCH 7/9] Wrap unwrapped last_build app doc --- corehq/apps/app_manager/models.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/corehq/apps/app_manager/models.py b/corehq/apps/app_manager/models.py index fd61230c6735..1ad0be2d859a 100644 --- a/corehq/apps/app_manager/models.py +++ b/corehq/apps/app_manager/models.py @@ -4478,18 +4478,14 @@ def check_build_dependencies(self, new_build): """ def has_dependencies(build): - if isinstance(build, Application): - profile = build.profile - else: - profile = build.get('profile', {}) - return bool( - profile.get('features', {}).get('dependencies') + build.profile.get('features', {}).get('dependencies') ) new_build_has_dependencies = has_dependencies(new_build) last_build = get_latest_build_doc(self.domain, self.id) + last_build = ApplicationBase.wrap(last_build) if last_build else None last_build_has_dependencies = has_dependencies(last_build) if last_build else False if not last_build_has_dependencies and new_build_has_dependencies: From 319593f8f2ee073af5d94c909455033cd477f644 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Mon, 9 Dec 2024 10:51:49 +0200 Subject: [PATCH 8/9] Use calling class's own wrap method --- corehq/apps/app_manager/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/corehq/apps/app_manager/models.py b/corehq/apps/app_manager/models.py index 1ad0be2d859a..988a73795467 100644 --- a/corehq/apps/app_manager/models.py +++ b/corehq/apps/app_manager/models.py @@ -4485,7 +4485,7 @@ def has_dependencies(build): new_build_has_dependencies = has_dependencies(new_build) last_build = get_latest_build_doc(self.domain, self.id) - last_build = ApplicationBase.wrap(last_build) if last_build else None + last_build = self.__class__.wrap(last_build) if last_build else None last_build_has_dependencies = has_dependencies(last_build) if last_build else False if not last_build_has_dependencies and new_build_has_dependencies: From 1c71a7b688a9c883760f48826cc897473ea85651 Mon Sep 17 00:00:00 2001 From: Charl Smit Date: Tue, 10 Dec 2024 08:36:53 +0200 Subject: [PATCH 9/9] Add some tests --- corehq/apps/app_manager/tests/test_apps.py | 45 ++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/corehq/apps/app_manager/tests/test_apps.py b/corehq/apps/app_manager/tests/test_apps.py index cf1ece2c393b..35f37c8fe656 100644 --- a/corehq/apps/app_manager/tests/test_apps.py +++ b/corehq/apps/app_manager/tests/test_apps.py @@ -414,3 +414,48 @@ def test_get_latest_enabled_build_with_loc_flag(self): location_flag_enabled=True, ) self.assertIsNone(build) + + def test_dependencies_feature_added(self): + factory = AppFactory(build_version='2.40.0') + m0, f0 = factory.new_basic_module('register', 'case') + f0.source = get_simple_form(xmlns=f0.unique_id) + factory.app.profile = {'features': {'dependencies': ['coffee']}} + factory.app.save() + self.addCleanup(factory.app.delete) + + with patch("corehq.apps.app_manager.models.metrics_counter") as metric_counter_mock: + factory.app.make_build() + metric_counter_mock.assert_called_with('commcare.app_build.dependencies_added') + + def test_dependencies_feature_removed(self): + factory = AppFactory(build_version='2.40.0') + m0, f0 = factory.new_basic_module('register', 'case') + f0.source = get_simple_form(xmlns=f0.unique_id) + factory.app.profile = {'features': {'dependencies': ['coffee']}} + factory.app.save() + self.addCleanup(factory.app.delete) + build1 = factory.app.make_build() + build1.save() + + factory.app.profile = {'features': {'dependencies': []}} + factory.app.save() + + with patch("corehq.apps.app_manager.models.metrics_counter") as metric_counter_mock: + factory.app.make_build() + metric_counter_mock.assert_called_with('commcare.app_build.dependencies_removed') + + def test_dependencies_feature_metrics_not_triggerd(self): + factory = AppFactory(build_version='2.40.0') + m0, f0 = factory.new_basic_module('register', 'case') + f0.source = get_simple_form(xmlns=f0.unique_id) + factory.app.profile = {'features': {'dependencies': []}} + factory.app.save() + self.addCleanup(factory.app.delete) + build1 = factory.app.make_build() + build1.save() + + factory.app.save() + + with patch("corehq.apps.app_manager.models.metrics_counter") as metric_counter_mock: + factory.app.make_build() + metric_counter_mock.assert_not_called()