ymcatwincities / openy

The Open Y platform. See README.md below
https://openy.org
GNU General Public License v3.0
49 stars 111 forks source link

openy_loc_branch has undeclared dependency to openy_node_alert #2462

Open froboy opened 3 years ago

froboy commented 3 years ago

https://github.com/ymcatwincities/openy/blob/40cabc3298e64297660f7a1ddcdb6bafc8137654/modules/openy_features/openy_location/modules/openy_loc_branch/src/BranchAlertBuilder.php#L6

This line references openy_node_alert, but https://github.com/ymcatwincities/openy/blob/40cabc3298e64297660f7a1ddcdb6bafc8137654/modules/openy_features/openy_location/modules/openy_loc_branch/openy_loc_branch.info.yml#L7 does not list that module as a dependency. This could result in unexpected errors if openy_loc_branch is enabled without openy_node_alert.

podarok commented 3 years ago

@froboy openy_node_alert is a dependency in OpenY profile. It can't be disabled as of now.

podarok commented 3 years ago

Hm, not true.. https://github.com/ymcatwincities/openy/search?l=YAML&q=openy_node_alert

Dependency management should become better with decoupling

froboy commented 3 years ago

Well... openy_node_alert is a part of the alerts package: https://github.com/ymcatwincities/openy/blob/40cabc3298e64297660f7a1ddcdb6bafc8137654/openy.packages.yml#L2-L7

The alerts package is installed with all three install types: https://github.com/ymcatwincities/openy/blob/40cabc3298e64297660f7a1ddcdb6bafc8137654/openy.installation_types.yml#L2-L4 https://github.com/ymcatwincities/openy/blob/40cabc3298e64297660f7a1ddcdb6bafc8137654/openy.installation_types.yml#L10-L13 https://github.com/ymcatwincities/openy/blob/40cabc3298e64297660f7a1ddcdb6bafc8137654/openy.installation_types.yml#L27-L32

So while it's installed with every Open Y installation, it's treated more like a soft dependency, so it could be uninstalled after install, which would then cause problems (much like we're seeing). :)

sarah-halby commented 2 years ago

@shelleygoetz do we have a jira ticket for this issue?

shelleygoetz commented 2 years ago

@sarah-halby we do, I must have missed putting the link in here. See: (https://openy.atlassian.net/browse/MAINTAIN-182)