Update minor fix 2#809
Conversation
Introduce get_series_plan_schedule_by_series_ids for lightweight plan fields and item counts, and update series service to use it. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Confidence Score: 4/5Safe to merge; the behavioural changes are intentional, well-tested, and confined to schedule calculation and group language fallback logic. All production call sites of
Reviews (1): Last reviewed commit: "Update series plan scheduling functional..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| schedule_plans = _get_sorted_active_plans( | ||
| plans, | ||
| published_only=True, | ||
| language=language, | ||
| fallback=fallback, | ||
| ) | ||
| series_total_days = sum(_plan_total_days(plan) for plan in sorted_plans) | ||
| first_plan = sorted_plans[0] | ||
| if not first_plan.start_date: | ||
| if not schedule_plans: | ||
| return None, None, series_total_days |
There was a problem hiding this comment.
published_only=True
Both production call sites pass published_only=True, so sorted_plans and schedule_plans are computed by identical _get_sorted_active_plans invocations. If sorted_plans is non-empty (the guard at line 184 already passed), schedule_plans will also be non-empty, making the if not schedule_plans return at line 194 dead code on that path. For the published_only=False path the two calls intentionally diverge (total_days from all plans, dates from published only), but for the common published_only=True path you pay the cost of a second filter+sort for no effect. You could short-circuit with schedule_plans = sorted_plans if published_only else _get_sorted_active_plans(plans, published_only=True, language=language, fallback=fallback) to make the intent explicit and avoid the duplicate work.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
2 participants
No description provided.