fix(grafana_datasource): orgId by name if defined to compare diff

[Nemental]

SUMMARY

If org_name is set instead of org_id, comparing the diff (after/before) to determine whether something has changed or not does not work.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

• module grafana_datasource

  ADDITIONAL INFORMATION

    "diff": {
        "after": {
            "access": "proxy",
            "basicAuth": false,
            "database": "",
            "isDefault": false,
            "jsonData": {
                "tlsAuth": false,
                "tlsAuthWithCACert": false
            },
            "name": "Loki",
            "orgId": 1, # <-- datasource left unchanged in orgId 6 but diff uses default value of org_id (1) to compare so it's "changed"
            "type": "loki",
            "url": "http://loki-stack-loki-stack:3100",
            "user": "",
            "withCredentials": false
        },
        "before": {
            "access": "proxy",
            "basicAuth": false,
            "database": "",
            "isDefault": false,
            "jsonData": {
                "tlsAuth": false,
                "tlsAuthWithCACert": false
            },
            "name": "Loki",
            "orgId": 6,
            "type": "loki",
            "url": "http://loki-stack-loki-stack:3100",
            "user": "",
            "withCredentials": false
        }
    },

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (879e206) 69.25% compared to head (90f9783) 70.82%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #345 +/- ## ========================================== + Coverage 69.25% 70.82% +1.56% ========================================== Files 9 18 +9 Lines 1184 1878 +694 Branches 263 326 +63 ========================================== + Hits 820 1330 +510 - Misses 212 404 +192 + Partials 152 144 -8 ``` | [Flag](https://app.codecov.io/gh/ansible-collections/community.grafana/pull/345/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ansible-collections) | Coverage Δ | | |---|---|---| | [integration](https://app.codecov.io/gh/ansible-collections/community.grafana/pull/345/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ansible-collections) | `69.45% <100.00%> (+0.19%)` | :arrow_up: | | [sanity](https://app.codecov.io/gh/ansible-collections/community.grafana/pull/345/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ansible-collections) | `23.53% <20.00%> (?)` | | | [units](https://app.codecov.io/gh/ansible-collections/community.grafana/pull/345/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ansible-collections) | `65.18% <20.00%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ansible-collections#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[rndmh3ro]

I actually liked your first try (https://github.com/ansible-collections/community.grafana/pull/345/commits/41596d36689fbd9e9f3ab94bffe08425fc3892c4) better, because there you didn't redefine params["org_id"].

And now that I think of it: Now we check twice, if org_name is set. Could we define a new function for this in the GrafanaInterface-class, e.g. get_org_id and use it at these two places?

Aside, for another issue: We obviously need tests for idempotency.

[Nemental]

@rndmh3ro thank you for your review! In the GrafanaInterface class is already a function called organization_by_name, which was implemented in #332. You can check my recent changes. Maybe it's a solution we can go with.

[rndmh3ro]

This is what I had in mind:

diff --git a/plugins/modules/grafana_datasource.py b/plugins/modules/grafana_datasource.py
index 910e16b..3784999 100644
--- a/plugins/modules/grafana_datasource.py
+++ b/plugins/modules/grafana_datasource.py
@@ -688,6 +688,7 @@ class GrafanaInterface(object):
     def __init__(self, module):
         self._module = module
         self.grafana_url = base.clean_url(module.params.get("url"))
+        self.org_id = None
         # {{{ Authentication header
         self.headers = {"Content-Type": "application/json"}
         if module.params.get("grafana_api_key", None):
@@ -698,12 +699,12 @@ class GrafanaInterface(object):
             self.headers["Authorization"] = basic_auth_header(
                 module.params["url_username"], module.params["url_password"]
             )
-            org_id = (
+            self.org_id = (
                 self.organization_by_name(module.params["org_name"])
                 if module.params["org_name"]
                 else module.params["org_id"]
             )
-            self.switch_organization(org_id)
+            self.switch_organization(self.org_id)
         # }}}

     def _send_request(self, url, data=None, headers=None, method="GET"):
@@ -923,12 +924,7 @@ def main():
     ds = grafana_iface.datasource_by_name(name)

     if state == "present":
-        org_id = (
-            grafana_iface.organization_by_name(module.params["org_name"])
-            if module.params["org_name"]
-            else module.params["org_id"]
-        )
-        payload = get_datasource_payload(module.params, org_id)
+        payload = get_datasource_payload(module.params, grafana_iface.org_id)
         if ds is None:
             grafana_iface.create_datasource(payload)
             ds = grafana_iface.datasource_by_name(name)

This way org_id is only defined in the init-function of the class. with self.org_id we can then use it outside of the class in the payload function.

I think, this should work. My local tests are failing with another error, so can you please test this?

[Nemental]

@rndmh3ro Yes it works :) Great suggestion to define the variable for org_id this way. Thank you.