Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
57 changes: 49 additions & 8 deletions static/js/app-dashboard/components/app.vue
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,14 @@
/>
</div>
<div class="tab-pane fade" id="conferences" role="tabpanel" aria-labelledby="conferences-tab">
<conferences-tab :conferences="conferences" />
<conferences-tab
:conferences="paginatedConferences"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve dashboard filters for conference list tab

Binding the list tab to paginatedConferences bypasses applyFilters(), which still only updates this.conferences based on the selected app version/browser/OS/country watchers. After this change, changing filters updates graphs but leaves the conference list unfiltered, which is a behavior regression from the previous wiring and makes the global filter UI inconsistent across tabs.

Useful? React with 👍 / 👎.

:total-count="conferencesCount"
:current-page="conferencesPage"
:per-page="conferencesPerPage"
:loading="conferencesLoading"
@page-changed="fetchConferences"
/>
</div>
</div>
</div>
Expand Down Expand Up @@ -147,6 +154,11 @@ export default {
data: {},

conferences: null,
paginatedConferences: null,
conferencesCount: 0,
conferencesPage: 1,
conferencesPerPage: 20,
conferencesLoading: false,
sessions: null,
connections: null,
issues: null,
Expand Down Expand Up @@ -179,8 +191,13 @@ export default {
},

async created() {
const since = new Date();
since.setDate(since.getDate() - peermetrics.daysHistory);
const created_at_gte = since.toISOString();

this.data.issues = await peermetrics.get(peermetrics.urls.issues(), {
appId: peermetrics.app.id
appId: peermetrics.app.id,
created_at_gte,
}).catch(e => {
console.warn(e)
});
Expand All @@ -190,9 +207,9 @@ export default {
}

this.data.conferences = await peermetrics.get(peermetrics.urls.conferences(), {
appId: peermetrics.app.id
})
.catch(e => {
appId: peermetrics.app.id,
created_at_gte,
}).catch(e => {
console.warn(e)
});

Expand All @@ -201,18 +218,22 @@ export default {
this.conferences = Object.freeze(this.data.conferences);
}

await this.fetchConferences();

this.data.sessions = await peermetrics.get(peermetrics.urls.sessions, {
appId: peermetrics.app.id
appId: peermetrics.app.id,
created_at_gte,
})
.catch(e => console.warn(e));

if(this.data.sessions) {
this.data.sessions = peermetrics.utils.populateIssues(this.data.sessions, this.data.issues)
this.sessions = Object.freeze(this.data.sessions);
}

this.data.connections = await peermetrics.get(peermetrics.urls.connections(), {
appId: peermetrics.app.id
appId: peermetrics.app.id,
created_at_gte,
})
.catch(e => {
console.warn(e)
Expand Down Expand Up @@ -291,6 +312,26 @@ export default {
return Object.values(object).sort((a, b) => a.value > b.value ? 1 : -1)
},

async fetchConferences(page) {
if (page) this.conferencesPage = page;
this.conferencesLoading = true;
const offset = (this.conferencesPage - 1) * this.conferencesPerPage;
try {
const response = await peermetrics.get(peermetrics.urls.conferences(), {
appId: peermetrics.app.id,
limit: this.conferencesPerPage,
offset: offset,
Comment on lines +320 to +323
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep pagination query scoped to dashboard history window

The new fetchConferences() request drops the created_at_gte filter that created() now applies to issues/sessions/connections and the full conference dataset, so the paginated conference tab can pull a different (all-time) dataset than the graphs and empty-state logic. On apps with older conferences, this causes inconsistent UI (e.g., graphs show only recent history while list/pagination counts include older rows) and unnecessary reads outside the intended time window.

Useful? React with 👍 / 👎.

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.

I believe the bot's concern is likely a false positive in our case.

The pagination is doing its job correctly:
The list shows X conferences at a time (one page) — that's expected
The charts show aggregated data across a broader dataset — also expected and intentional

These two views are supposed to show different scopes.

});
if (response) {
this.paginatedConferences = Object.freeze(response.results || []);
this.conferencesCount = response.count || 0;
}
} catch (e) {
console.warn(e);
}
this.conferencesLoading = false;
},

applyFilters () {
// if no filters are applied, use all the data
if (!this.selectedAppVersion && !this.selectedBrowser && !this.selectedOs && !this.selectedCountry) {
Expand Down
74 changes: 41 additions & 33 deletions static/js/app-dashboard/components/conferencesTab.vue
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@
</div>
<div class="row mt-3">
<div class="col">
<div id="conference-list-holder" class="list-group list-group-flush list-custom">
<div v-if="loading" class="text-center py-4">
<span class="text-muted">Loading conferences...</span>
</div>
<div v-else id="conference-list-holder" class="list-group list-group-flush list-custom">
<a
v-for="conference in conferencesPagination"
v-for="conference in conferences"
:key="conference.id"
:href="createPath(conference)"
class="list-group-item list-group-item-action"
Expand All @@ -18,32 +21,27 @@
<span class="text-muted">{{ conference.conference_id }}</span>
</div>

<i
v-if="hasError(conference)"
<i
v-if="conference.has_errors"
class="icon-cross text-danger"
title="We detected errors for this conference."
data-toggle="tooltip"
data-placement="top"
@mouseover="tooltipHover"
></i>
<i
v-else-if="hasWarning(conference)"
></i>
<i
v-else-if="conference.has_warnings"
class="icon-warning text-warning"
title="There are some warnings for this conference."
data-toggle="tooltip"
data-placement="top"
@mouseover="tooltipHover"
></i>
></i>
</a>
</div>

<b-pagination
v-if="conferences && conferences.length > perPage"
:total-rows="rows"
v-model="currentPage"
v-if="totalCount > perPage"
:total-rows="totalCount"
:value="currentPage"
:per-page="perPage"
aria-controls="conference-list-holder"
class="mt-3"
@input="onPageChange"
></b-pagination>
</div>
</div>
Expand All @@ -52,7 +50,6 @@

<script>
import { BPagination } from "bootstrap-vue";
import conferencesFunctions from "../../mixins/conferences"
export default {
name: "conferences-tab",
props: {
Expand All @@ -61,27 +58,38 @@ export default {
validator: value => {
return Array.isArray(value) || peermetrics.utils.isNull(value)
}
}
},
totalCount: {
type: Number,
default: 0,
},
currentPage: {
type: Number,
default: 1,
},
perPage: {
type: Number,
default: 20,
},
loading: {
type: Boolean,
default: false,
},
},
components: {
BPagination
},
mixins: [conferencesFunctions],

computed: {
rows() {
if(!this.conferences) return 0
return this.conferences.length;
}
},

methods: {
// TODO: hack. find the right solution for this
// move away from jq in the future. this is the only way to force tooltips to appear
// tried with <b-icon> but they use SVGs. we use css icons now
tooltipHover(ev) {
$(ev.target).tooltip('show')
}
hasName(conference) {
return !!conference.conference_name;
},
createPath(conference) {
return peermetrics.createPath('conference', conference.id)
},
onPageChange(page) {
this.$emit('page-changed', page);
},
},
};
</script>
Expand Down
4 changes: 2 additions & 2 deletions static/js/mixins/conferences.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ const conferencesFunctions = {
return !!conference.conference_name;
},
hasError(conference) {
return conference.issues.some((issue) => issue.type === 'error')
return conference.issues && conference.issues.some((issue) => issue.type === 'error')
},
hasWarning(conference) {
return conference.issues.some((issue) => issue.type === 'warning')
return conference.issues && conference.issues.some((issue) => issue.type === 'warning')
},
createPath(conference) {
return peermetrics.createPath('conference', conference.id)
Expand Down
2 changes: 1 addition & 1 deletion static/js/participant/components/conferencesTab.vue
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export default {
// if an sfu, show all conferences that had at least one issue
if (this.isSfu) {
return this.conferences.filter((conference) => {
return conference.issues.length
return conference.issues && conference.issues.length
})
}

Expand Down