Skip to content

Move "For me" filter to server-side to fix table pagination #384

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
frenzymadness wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Move "For me" filter to server-side to fix table pagination #384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 47 additions & 1 deletion app/routes/events/crud.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from app.extensions import db
from app.models.assignment import Assignment
from app.models.equipment import EquipmentCategory, EquipmentItem, EquipmentItemStatus, EquipmentType
from app.models.event import Event, EventSpot, EventStatus, EventTemplate, EventType
from app.models.event import Event, EventSpot, EventStatus, EventTemplate, EventType, spot_qualifications
from app.models.master_event import MasterEvent
from app.models.qualification import Qualification
from app.models.user import UserAccount
Expand Down Expand Up @@ -86,6 +86,8 @@ def _parse_index_filters() -> dict:
raw_types = request.args.get("types", "")
active_types = [t for t in raw_types.split(",") if t in _ALL_EVENT_TYPES]

for_me = request.args.get("for_me") == "1" and current_user.has_permission("event.assign_own")

return {
"show_archived": show_archived,
"page": page,
Expand All @@ -94,6 +96,7 @@ def _parse_index_filters() -> dict:
"sort_dir": sort_dir,
"active_me": active_me,
"active_types": active_types,
"for_me": for_me,
}


Expand Down Expand Up @@ -168,6 +171,44 @@ def _build_eligible_spot_map(events: list[Event]) -> dict[int, list[tuple[int, s
return result


def _eligible_event_ids_for_user(user: UserAccount) -> list[int]:
"""Return a list of event IDs where the user has at least one unoccupied, fillable spot."""
fillable_ids = user_fillable_qual_ids(user)

# Find deleted qual IDs so we can ignore them (match existing eligibility logic)
deleted_qual_ids = {
q.id for q in db.session.scalars(db.select(Qualification).where(Qualification.is_deleted.is_(True))).all()
}

# Fetch all (spot_id, event_id, qual_id) for unoccupied spots
rows = db.session.execute(
db.select(
EventSpot.id.label("spot_id"),
EventSpot.event_id,
spot_qualifications.c.qualification_id,
)
.outerjoin(Assignment, Assignment.spot_id == EventSpot.id)
.outerjoin(spot_qualifications, spot_qualifications.c.spot_id == EventSpot.id)
.where(Assignment.id.is_(None))
).all()
Comment on lines +174 to +193

@coderabbitai coderabbitai Bot Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Align for_me eligibility with claimable-event status.

_eligible_event_ids_for_user() currently considers all events with unoccupied eligible spots, but the rest of the page contract (notably _build_eligible_spot_map) treats eligibility as claimable spots in ASSIGNMENTS_OPEN. This can surface non-claimable events under “Pro mě”.

Suggested fix 
 def _eligible_event_ids_for_user(user: UserAccount) -> list[int]:
@@
     rows = db.session.execute(
         db.select(
             EventSpot.id.label("spot_id"),
             EventSpot.event_id,
             spot_qualifications.c.qualification_id,
         )
+        .join(Event, Event.id == EventSpot.event_id)
         .outerjoin(Assignment, Assignment.spot_id == EventSpot.id)
         .outerjoin(spot_qualifications, spot_qualifications.c.spot_id == EventSpot.id)
-        .where(Assignment.id.is_(None))
+        .where(
+            Assignment.id.is_(None),
+            Event.status == EventStatus.ASSIGNMENTS_OPEN,
+        )
     ).all()

Also applies to: 205-207

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/routes/events/crud.py` around lines 174 - 193, The function
_eligible_event_ids_for_user currently returns events with any unoccupied
eligible spot but doesn’t enforce that the event is in ASSIGNMENTS_OPEN (which
_build_eligible_spot_map expects for "for_me"); update the DB query in
_eligible_event_ids_for_user to join Event and add a where clause restricting
Event.state == ASSIGNMENTS_OPEN (use the same enum/constant used elsewhere),
still filter out deleted quals and only include qualifications in fillable_ids,
and apply the same change to the analogous logic around lines 205-207 so both
places consistently consider only claimable events.


spot_event: dict[int, int] = {}
spot_quals: dict[int, set[int]] = {}
for spot_id, event_id, qual_id in rows:
spot_event[spot_id] = event_id
if spot_id not in spot_quals:
spot_quals[spot_id] = set()
if qual_id is not None and qual_id not in deleted_qual_ids:
spot_quals[spot_id].add(qual_id)

eligible_event_ids: set[int] = set()
for spot_id, required_qual_ids in spot_quals.items():
if all(qid in fillable_ids for qid in required_qual_ids):
eligible_event_ids.add(spot_event[spot_id])

return list(eligible_event_ids) if eligible_event_ids else [-1]


# ── List ──────────────────────────────────────────────────────────────────────


Expand Down Expand Up @@ -200,6 +241,10 @@ def index() -> str:
else:
query = query.where(db.false())

if f["for_me"]:
eligible_ids = _eligible_event_ids_for_user(current_user)
query = query.where(Event.id.in_(eligible_ids))

query = _apply_index_order(query, f["sort_col"], f["sort_dir"])
pagination = db.paginate(query, page=f["page"], per_page=PER_PAGE, error_out=False)
events = pagination.items
Expand Down Expand Up @@ -232,6 +277,7 @@ def index() -> str:
eligible_spot_map=_build_eligible_spot_map(events),
active_named_mes=active_named_mes,
status_colors=STATUS_BADGE_COLORS,
for_me=f["for_me"],
)


Expand Down
59 changes: 3 additions & 56 deletions app/static/js/events-index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,16 @@
var ACTIVE_STATUSES = cfg.activeStatuses || [];
var CLAIM_BASE = cfg.claimBase || "";
var ACTIVE_ME_NAME = cfg.activeMeName || "";
var FOR_ME = cfg.forMe || false;

var STORAGE_VIEW = "medcover_events_view";
var STORAGE_ELIG = "medcover_events_elig";
var STORAGE_DATE = "medcover_events_cal_date";

var calendarInitialized = false;
var calendar = null;
var allCalendarEvents = null;
var eligFilter = false;
var currentCalDate = null;

// ── Per-page JS filter (elig only — status + ME are server-side) ──

function loadEligFilter() {
try { return localStorage.getItem(STORAGE_ELIG) === "1"; } catch(e) { return false; }
}
function saveEligFilter(v) { localStorage.setItem(STORAGE_ELIG, v ? "1" : "0"); }

function loadCalendarDate() {
try { return localStorage.getItem(STORAGE_DATE) || null; } catch(e) { return null; }
}
Expand All @@ -42,19 +34,9 @@
return d.getFullYear() + '-' + (m < 10 ? '0' : '') + m + '-' + (day < 10 ? '0' : '') + day;
}

// ── Table row visibility (elig only — status + ME are server-side) ──
// ── Table row visibility (status + ME + elig are all server-side) ──

function applyLocalFilters() {
var tbody = document.querySelector("#events-table tbody");
if (!tbody) return;
var visibleCount = 0;
tbody.querySelectorAll("tr").forEach(function (row) {
var eligOk = !eligFilter || row.dataset.eligible === "1";
row.style.display = eligOk ? "" : "none";
if (eligOk) visibleCount++;
});
var emptyMsg = document.getElementById("table-empty-msg");
if (emptyMsg) emptyMsg.classList.toggle('d-none', visibleCount > 0);
if (calendarInitialized && calendar) calendar.refetchEvents();
_saveEventNav();
}
Expand All @@ -76,17 +58,6 @@
} catch(e) {}
}

function toggleEligFilter() {
eligFilter = !eligFilter;
saveEligFilter(eligFilter);
var btn = document.getElementById("btn-elig-filter");
if (btn) {
btn.classList.toggle("active", eligFilter);
btn.blur();
}
applyLocalFilters();
}

// ── ME filter navigation (server-side; select triggers URL change) ──────

function navigateToMe(meId) {
Expand Down Expand Up @@ -138,7 +109,7 @@
}
successCallback(allCalendarEvents.filter(function (e) {
var statusOk = ACTIVE_STATUSES.includes(e.extendedProps.status_key);
var eligOk = !eligFilter || e.extendedProps.eligible;
var eligOk = !FOR_ME || e.extendedProps.eligible;
var meOk = !ACTIVE_ME_NAME || (e.extendedProps.me_name || "") === ACTIVE_ME_NAME;
return statusOk && eligOk && meOk;
}));
Expand Down Expand Up @@ -240,29 +211,6 @@
// ── Init ──────────────────────────────────────────────────────────────────

document.addEventListener("DOMContentLoaded", function () {
eligFilter = loadEligFilter();
var eligBtn = document.getElementById("btn-elig-filter");
if (eligBtn) {
eligBtn.classList.toggle("active", eligFilter);
var eligTouchStartY = 0;
var eligTouchFired = false;
eligBtn.addEventListener("touchstart", function (e) {
eligTouchStartY = e.touches[0].clientY;
}, { passive: true });
eligBtn.addEventListener("touchend", function (e) {
var dy = Math.abs(e.changedTouches[0].clientY - eligTouchStartY);
if (dy > 10) return;
eligTouchFired = true;
e.preventDefault();
toggleEligFilter();
setTimeout(function () { eligTouchFired = false; }, 500);
}, { passive: false });
eligBtn.addEventListener("click", function () {
if (eligTouchFired) return;
toggleEligFilter();
});
}

applyLocalFilters();

var saved = localStorage.getItem(STORAGE_VIEW) || "table";
Expand Down Expand Up @@ -329,5 +277,4 @@
window.navigateToMe = navigateToMe;
window.clearSelection = clearSelection;
window.submitBulk = submitBulk;
window.toggleEligFilter = toggleEligFilter;
})();
14 changes: 7 additions & 7 deletions app/templates/events/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
sort=sort_col if sort_col != 'start' else None,
dir=sort_dir if sort_col != 'start' or sort_dir != 'asc' else None,
archived=1 if (archived if archived is not none else show_archived) else None,
for_me=1 if for_me else None,
) -}}
{%- endmacro %}

Expand Down Expand Up @@ -68,7 +69,7 @@ <h2 class="mb-0">Akce {{ help_icon("Akce procházejí těmito fázemi:\n• Konc
{%- else -%}
{%- set new_statuses = active_statuses + [toggle_status] -%}
{%- endif -%}
{{ url_for('events.index', statuses=new_statuses|join(','), types=active_types|join(',') if active_types|length < all_event_types|length else None, me_id=active_me.id if active_me else None, sort=sort_col if sort_col != 'start' else None, dir=sort_dir if sort_col != 'start' or sort_dir != 'asc' else None, archived=1 if show_archived else None) }}
{{ url_for('events.index', statuses=new_statuses|join(','), types=active_types|join(',') if active_types|length < all_event_types|length else None, me_id=active_me.id if active_me else None, sort=sort_col if sort_col != 'start' else None, dir=sort_dir if sort_col != 'start' or sort_dir != 'asc' else None, archived=1 if show_archived else None, for_me=1 if for_me else None) }}
{%- endmacro %}

{# Type filter toggle macro #}
Expand All @@ -79,7 +80,7 @@ <h2 class="mb-0">Akce {{ help_icon("Akce procházejí těmito fázemi:\n• Konc
{%- else -%}
{%- set new_types = active_types + [toggle_type] -%}
{%- endif -%}
{{ url_for('events.index', types=new_types|join(',') if new_types|length < all_event_types|length else None, statuses=active_statuses|join(','), me_id=active_me.id if active_me else None, sort=sort_col if sort_col != 'start' else None, dir=sort_dir if sort_col != 'start' or sort_dir != 'asc' else None, archived=1 if show_archived else None) }}
{{ url_for('events.index', types=new_types|join(',') if new_types|length < all_event_types|length else None, statuses=active_statuses|join(','), me_id=active_me.id if active_me else None, sort=sort_col if sort_col != 'start' else None, dir=sort_dir if sort_col != 'start' or sort_dir != 'asc' else None, archived=1 if show_archived else None, for_me=1 if for_me else None) }}
{%- endmacro %}

{# Sort URL: preserve statuses + types + me_id + archived, toggle sort col/dir #}
Expand All @@ -88,7 +89,7 @@ <h2 class="mb-0">Akce {{ help_icon("Akce procházejí těmito fázemi:\n• Konc
{%- if sort_col == col -%}
{%- set new_dir = 'desc' if sort_dir == 'asc' else 'asc' -%}
{%- endif -%}
{{ url_for('events.index', sort=col, dir=new_dir, statuses=active_statuses|join(','), types=active_types|join(',') if active_types|length < all_event_types|length else None, me_id=active_me.id if active_me else None, archived=1 if show_archived else None) }}
{{ url_for('events.index', sort=col, dir=new_dir, statuses=active_statuses|join(','), types=active_types|join(',') if active_types|length < all_event_types|length else None, me_id=active_me.id if active_me else None, archived=1 if show_archived else None, for_me=1 if for_me else None) }}
{%- endmacro %}
{% macro sort_icon(col) %}
{%- if sort_col == col -%}
Expand All @@ -108,7 +109,8 @@ <h2 class="mb-0">Akce {{ help_icon("Akce procházejí těmito fázemi:\n• Konc
<a href="{{ status_url('COMPLETED') }}" class="btn btn-sm filter-btn-neutral filter-btn {{ 'active' if 'COMPLETED' in active_statuses }}">Dokončena</a>
<a href="{{ status_url('CANCELLED') }}" class="btn btn-sm btn-outline-secondary filter-btn {{ 'active' if 'CANCELLED' in active_statuses }}">Zrušena</a>
{% if current_user.has_permission('event.assign_own') %}
<button type="button" class="btn btn-sm btn-outline-info filter-btn ms-2" id="btn-elig-filter">Pro mě</button>
<a href="{{ url_for('events.index', statuses=active_statuses|join(','), types=active_types|join(',') if active_types|length < all_event_types|length else None, me_id=active_me.id if active_me else None, sort=sort_col if sort_col != 'start' else None, dir=sort_dir if sort_col != 'start' or sort_dir != 'asc' else None, archived=1 if show_archived else None, for_me=None if for_me else 1) }}"
class="btn btn-sm btn-outline-info filter-btn ms-2 {{ 'active' if for_me }}">Pro mě</a>
{% endif %}
<select id="me-filter-select" class="form-select form-select-sm ms-2 maxw-250 {{ 'd-none' if not active_named_mes }}"
title="Filtrovat podle nadřazené akce">
Expand Down Expand Up @@ -271,8 +273,6 @@ <h2 class="mb-0">Akce {{ help_icon("Akce procházejí těmito fázemi:\n• Konc
</tbody>
</table>
</div>
<p class="text-muted small d-none" id="table-empty-msg">Žádné akce neodpovídají aktivním filtrům.</p>

{# ── Pagination ─────────────────────────────────────────────────────────── #}
{% if pagination.pages > 1 %}
<nav aria-label="Stránkování akcí" class="mt-2">
Expand Down Expand Up @@ -344,6 +344,6 @@ <h5 class="modal-title fs-6" id="spotPickModalLabel">Vyberte pozici</h5>
}
</style>

<script type="application/json" id="events-page-cfg">{"feedUrl": "{{ url_for('events.feed') }}{{ '?archived=1' if show_archived }}", "hasDraftPerm": {{ 'true' if has_draft_perm else 'false' }}, "activeStatuses": {{ active_statuses | tojson }}, "claimBase": "{{ url_for('assignments.claim', spot_id=0) | replace('/0', '/') }}", "activeMeName": {{ (active_me.name if active_me else '') | tojson }}}</script>
<script type="application/json" id="events-page-cfg">{"feedUrl": "{{ url_for('events.feed') }}{{ '?archived=1' if show_archived }}", "hasDraftPerm": {{ 'true' if has_draft_perm else 'false' }}, "activeStatuses": {{ active_statuses | tojson }}, "claimBase": "{{ url_for('assignments.claim', spot_id=0) | replace('/0', '/') }}", "activeMeName": {{ (active_me.name if active_me else '') | tojson }}, "forMe": {{ for_me | tojson }}}</script>
<script src="{{ url_for('static', filename='js/events-index.js') }}?v={{ static_ver }}"></script>
{% endblock %}
Loading
Loading