Refactor _list_pools.py so all methods that handle D-Bus properties are grouped in Default parent class

[mulkieran]

No description provided.

[mulkieran]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Walkthrough

The PR refactors alert computation logic in the pool listing functionality by renaming DefaultAlerts to DeviceSizeChangedAlerts, changing the constructor parameter from devs to devs_to_search, extracting the instance method alert_codes into a standalone _alert_codes helper function, and updating all call sites to consolidate device-size-change and general pool alerts through both sources.

Changes

Cohort / File(s)

Summary

Alert Computation Refactoring src/stratis_cli/_actions/_list_pool.py

Renamed DefaultAlerts class to DeviceSizeChangedAlerts; updated constructor parameter signature; extracted alert_codes method into standalone _alert_codes function handling availability, no-alloc space, and encryption alerts; modified internal storage attributes from increased/decreased to match new parameter devs_to_search; updated all call sites in detail and table rendering to combine alerts from both _alert_codes() and alerts.alert_codes() with sorting.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Add type annotations for existing methods in _list_pool.py #1267: Modifies type annotations and signatures for the DefaultAlerts class in the same file, directly affected by this PR's class rename and API changes.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The PR title mentions refactoring to group D-Bus methods in a parent class, but the actual changes introduce DeviceSizeChangedAlerts, remove DefaultAlerts, and refactor alert handling logic—not the described D-Bus grouping.	Update the PR title to accurately reflect the main changes, such as: 'Refactor alert handling in _list_pools.py to separate device-size-change and pool alerts' or similar.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Tip

You can get early access to new features in CodeRabbit.

Enable the early_access setting to enable early access features such as new models, tools, and more.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/stratis_cli/_actions/_list_pool.py (1)

313-322: ⚠️ Potential issue | 🟡 Minor

Update stale docstring type for alerts parameter.

Line 321 still documents DefaultAlerts, but the signature now takes DeviceSizeChangedAlerts.

✏️ Suggested docstring fix

-        :param DefaultAlerts alerts: pool alerts
+        :param DeviceSizeChangedAlerts alerts: pool alerts

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/stratis_cli/_actions/_list_pool.py` around lines 313 - 322, The docstring
for the method with signature (self, pool_object_path: str, mopool: Any, alerts:
DeviceSizeChangedAlerts) incorrectly documents the alerts parameter as
DefaultAlerts; update the docstring to reference DeviceSizeChangedAlerts (or the
correct type alias/class) for the alerts parameter and adjust any param
description text to match the new type name so the signature and docstring are
consistent.

🧹 Nitpick comments (1)

src/stratis_cli/_actions/_list_pool.py (1)

328-331: Consider centralizing combined alert formatting logic.

Both detail and table paths manually combine alert lists. A tiny helper for combining/sorting would reduce drift risk between views.

♻️ Optional refactor sketch

+def _combined_alerts(
+    pool_object_path: str, mopool: Any, alerts: DeviceSizeChangedAlerts
+) -> list[Any]:
+    return _alert_codes(mopool) + alerts.alert_codes(pool_object_path)
+
@@
-        alert_summary = sorted(
-            f"{code}: {code.summarize()}"
-            for code in alerts.alert_codes(pool_object_path) + _alert_codes(mopool)
-        )
+        alert_summary = sorted(
+            f"{code}: {code.summarize()}"
+            for code in _combined_alerts(pool_object_path, mopool, alerts)
+        )
@@
-                        for code in (
-                            _alert_codes(mopool) + alerts.alert_codes(pool_object_path)
-                        )
+                        for code in _combined_alerts(pool_object_path, mopool, alerts)

Also applies to: 529-531

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/stratis_cli/_actions/_list_pool.py` around lines 328 - 331, The two
places building alert_summary duplicate logic by combining
alerts.alert_codes(pool_object_path) and _alert_codes(mopool) and
sorting/formatting them; extract that into a small helper (e.g., a function
named format_combined_alerts or get_sorted_alert_summaries) and replace the
inline generator expressions in both locations (the alert_summary assignment at
the current block and the one around lines 529-531) to call this helper so the
combination, formatting (f"{code}: {code.summarize()}"), and sorting are
centralized and consistent.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/stratis_cli/_actions/_list_pool.py`:
- Around line 313-322: The docstring for the method with signature (self,
pool_object_path: str, mopool: Any, alerts: DeviceSizeChangedAlerts) incorrectly
documents the alerts parameter as DefaultAlerts; update the docstring to
reference DeviceSizeChangedAlerts (or the correct type alias/class) for the
alerts parameter and adjust any param description text to match the new type
name so the signature and docstring are consistent.

---

Nitpick comments:
In `@src/stratis_cli/_actions/_list_pool.py`:
- Around line 328-331: The two places building alert_summary duplicate logic by
combining alerts.alert_codes(pool_object_path) and _alert_codes(mopool) and
sorting/formatting them; extract that into a small helper (e.g., a function
named format_combined_alerts or get_sorted_alert_summaries) and replace the
inline generator expressions in both locations (the alert_summary assignment at
the current block and the one around lines 529-531) to call this helper so the
combination, formatting (f"{code}: {code.summarize()}"), and sorting are
centralized and consistent.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 36a6a080-ffcc-40fd-acf4-381bc32b25ab

📥 Commits

Reviewing files that changed from the base of the PR and between 84e9033 and 906f313.

📒 Files selected for processing (1)

• src/stratis_cli/_actions/_list_pool.py