Skip to content
This repository has been archived by the owner on Mar 28, 2023. It is now read-only.

Refactor mod loading #1859

Closed
wants to merge 10 commits into from
Closed

Refactor mod loading #1859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c0714ce
fixLoadingStates
Jan 9, 2020
d2a4cc2
Merge branch 'master' into refactorModLoading
Jan 13, 2020
53a3348
refactor excluding own peerID
Jan 13, 2020
76935bd
only set loading state if actually loading
Jan 14, 2020
81a1ecc
clean up language
Jan 14, 2020
b67acea
remove unneccessary calls
Jan 14, 2020
1b89467
tweak language
Jan 14, 2020
4a9b265
remove cleverness with excluded id, fix bug when no currency set
Jan 14, 2020
9fbba4e
remove superfluous code
Jan 14, 2020
033e131
cleanup
Jan 14, 2020
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
12 changes: 6 additions & 6 deletions js/languages/en_US.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1517,20 +1517,20 @@
"moderatorsLoaded": "%{total} moderator loaded |||| %{total} moderators loaded.",
"noModsMsg": {
"noModerators": {
"title": "No Valid Moderators Are Available.",
"body": "None of the loaded moderators are valid. This order will be a direct payment."
"title": "No valid moderators have been loaded.",
"body": "This order will be a direct payment."
},
"noVerifiedModerators": {
"title": "No Verified Moderators Available",
"title": "No verified moderators have been loaded.",
"body": "Try viewing unverified moderators."
},
"noMatchingModerators": {
"title": "No moderators that accept %{coin} are available.",
"title": "No moderators that accept %{coin} have been loaded.",
"body": "Moderators are available on one or more of the other currencies accepted by this listing."
},
"noMatchingVerifiedModerators": {
"title": "No verified moderators that accept %{coin} have been loaded.",
"body": "Try viewing unverified moderators."
"body": "Try viewing unverified moderators or using another currency."
}
},
"showUnverified": "Show Unverified Moderators",
Expand Down Expand Up @@ -5740,4 +5740,4 @@
"zu": "Zulu",
"zu-ZA": "Zulu (South Africa)"
}
}
}
15 changes: 5 additions & 10 deletions js/templates/components/moderators/moderators.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
<div class="moderatorsWrapper clrBr <% print(ob.wrapperClasses) %> js-moderatorsWrapper">
<% // placeholder so border collapse doesn't erase the table border when there are no table cells %>
<% if (ob.placeholder) { %>
<div class="moderatorCard clrBr">
<div class="moderatorCardInner">
</div>
</div>
<% } %>
<% if (ob.totalIDs && !ob.totalShown) { %>
Copy link
Contributor Author

@jjeffryes jjeffryes Jan 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

The placeholder got refactored so the messages below take care of everything with less duplication. This also makes sure there's always a message.

<% if (!ob.totalShown) { %>
<div class="moderatorCard moderatorsMessage clrBr">
<div class="moderatorCardInner">
<div class="flexCent">
Expand All @@ -15,10 +8,12 @@
const showOnlyCur = ob.showOnlyCur ? 'Matching' : '';
const opts = ob.showOnlyCur ? { coin: ob.showOnlyCur } : {};
const msgPath = `moderators.noModsMsg.no${showOnlyCur}${showOnlyVer}Moderators`;
const title = ob.loading ?
ob.polyT('moderators.moderatorsLoading') : ob.polyT(`${msgPath}.title`, opts);
%>
<h4><%= ob.polyT(`${msgPath}.title`, opts) %></h4>
<h4><%= title %></h4>
<% // The section below is only relevant if the moderators are loaded in a purchasing context. %>
<% if (ob.purchase) { %>
<% if (ob.purchase && !ob.loading) { %>
<div class="tx4 clrT2"><%= ob.polyT(`${msgPath}.body`) %></div>
<% if (showOnlyVer) { %>
<div class="padTn"><% // just a spacer %></div>
Expand Down
2 changes: 1 addition & 1 deletion js/templates/components/moderators/status.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<% if (ob.showSpinner && ob.loading) { %>
<%= ob.spinner({ className: 'spinnerTxt js-spinner' }) %>
<% } %>
<span class="clrT4">
<span class="clrTEmph1">
<% if (ob.mode === 'loaded') {
print(ob.polyT('moderators.moderatorsLoaded', { total: ob.total, smart_count: ob.total }));
} else if (ob.mode === 'loadingXofY') {
Expand Down
60 changes: 28 additions & 32 deletions js/views/components/moderators/Moderators.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ export default class extends baseVw {

super(opts);
this.options = opts;
this.excludeIDs = opts.excludeIDs;
this.excludeIDs = [...opts.excludeIDs, app.profile.id];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seemed a lot smarter just include the user's own profile here, instead of adding it later.

We could add a parameter to not exclude it in the future if that use case ever came up.

this.modsToFetch = [...opts.moderatorIDs];
this.unfetchedMods = [];
this.fetchingMods = [];
this.fetchingVerifiedMods = [];
this.modFetches = [];
this.moderatorsCol = new Moderators();
Expand Down Expand Up @@ -172,37 +172,36 @@ export default class extends baseVw {
throw new Error('Please provide the list of moderators as an array.');
}

// don't get any that have already been added or excluded, or the user's own id.
const excluded = [app.profile.id, ...this.allIDs, ...this.excludeIDs];
const IDs = _.without(op.moderatorIDs, excluded);
const includeString = op.include ? `&include=${op.include}` : '';
const urlString =
`ob/${op.apiPath}?async=${op.async}${includeString}&usecache=${op.useCache}`;
const url = app.getServerUrl(urlString);

this.unfetchedMods = IDs;
this.fetchingMods = IDs;
this.fetchingVerifiedMods = app.verifiedMods.matched(IDs);

this.setState({
loading: true,
noValidModerators: false,
noValidVerifiedModerators: !this.fetchingVerifiedMods.length,
});
// don't get any that have already been added or excluded.
const excluded = [...this.allIDs, ...this.excludeIDs];
this.modsToFetch = _.without(op.moderatorIDs, excluded);
this.unfetchedMods = [...this.modsToFetch];
this.fetchingVerifiedMods = app.verifiedMods.matched(this.modsToFetch);

// Either a list of IDs can be posted, or any available moderators can be retrieved with GET
if (IDs.length || op.method === 'GET') {
if (this.modsToFetch.length || op.method === 'GET') {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was moved so it wouldn't set the state to loading if there were no mods to fetch.

this.setState({
loading: true,
noValidModerators: false,
noValidVerifiedModerators: !this.fetchingVerifiedMods.length,
});

this.moderatorsStatus.setState({
hidden: false,
loaded: 0,
toLoad: IDs.length,
toLoad: this.modsToFetch.length,
total: this.modCount,
loading: true,
});

const fetch = $.ajax({
url,
data: JSON.stringify(IDs),
data: JSON.stringify(this.modsToFetch),
method: op.method,
})
.done((data) => {
Expand All @@ -213,13 +212,15 @@ export default class extends baseVw {
const eventData = event.jsonData;
if (eventData.error) {
// errors don't have a message id, check to see if the peerID matches
if (IDs.includes(eventData.peerId)) {
// provide the expected capitalization of peerID
eventData.peerID = eventData.peerId;
delete eventData.peerId;
if (this.modsToFetch.includes(eventData.peerID)) {
this.processMod(eventData);
}
} else if (eventData.id === socketID && !excluded.includes(eventData.peerId)) {
} else if (
eventData.id &&
eventData.peerId &&
eventData.id === socketID &&
!excluded.includes(eventData.peerId)
) {
this.processMod(eventData.profile);
}
});
Expand All @@ -230,9 +231,7 @@ export default class extends baseVw {
data.forEach(mod => {
if (!excluded.includes(mod.peerId)) this.processMod(mod.profile);
});
this.unfetchedMods = [];
this.checkNotFetched();
if (!data.length) this.trigger('noModsFound', { guids: IDs });
if (!data.length) this.trigger('noModsFound', { guids: this.modsToFetch });
}
})
.fail((xhr) => {
Expand All @@ -259,7 +258,7 @@ export default class extends baseVw {
}

checkNotFetched() {
if (this.unfetchedMods.length === 0 && this.fetchingMods.length) {
if (this.unfetchedMods.length === 0) {
// All ids have been fetched and ids existed to fetch.
this.moderatorsStatus.setState({
loading: false,
Expand All @@ -272,7 +271,7 @@ export default class extends baseVw {
// Either ids are still fetching, or this is an open fetch with no set ids.
this.moderatorsStatus.setState({
loaded: this.moderatorsCol.length, // not shown if open fetch
toLoad: this.fetchingMods.length, // not shown if open fetch
toLoad: this.modsToFetch.length, // not shown if open fetch
total: this.modCount,
});
// re-render to show the unverified moderators button if needed.
Expand Down Expand Up @@ -403,19 +402,16 @@ export default class extends baseVw {
render() {
const state = this.getState();
const showMods = this.modCards.filter(mod => this.modShouldRender(mod.model));
const unVerCount = this.modCards.filter(mod =>
mod.model.hasModCurrency(state.showOnlyCur) && !mod.model.isVerified).length;
const totalIDs = this.allIDs.length;
const unVerCount = this.modCards.filter(mod => (!state.showOnlyCur ||
mod.model.hasModCurrency(state.showOnlyCur)) && !mod.model.isVerified).length;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since a default currency isn't being passed in (removed below), this was updated to be count all the unverified moderators if there is no currency set.

clearTimeout(this.renderTimer);
this.renderTimer = null;

loadTemplate('components/moderators/moderators.html', t => {
this.$el.html(t({
wrapperClasses: this.options.wrapperClasses,
placeholder: !showMods.length && (this.unfetchedMods.length || !totalIDs),
purchase: this.options.purchase,
totalShown: showMods.length,
totalIDs,
unVerCount,
...state,
}));
Expand Down
2 changes: 1 addition & 1 deletion js/views/components/moderators/Status.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export default class extends BaseVw {
let mode = this.getState().mode;
if (mode === 'loadingXofY') mode = 'loadingXofYTimedOut';
this.setState({ showSpinner: false, mode });
}, 10000);
}, 60000);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made this longer since I was seeing the message for no mods appear then mods would load, which was pretty clunky.

}
super.setState(state, options);
}
Expand Down
11 changes: 0 additions & 11 deletions js/views/modals/Settings/Store.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,22 +286,11 @@ export default class extends baseVw {
const remAvail = this.modsAvailable.removeModeratorsByID(this.modsAvailable.selectedIDs);

this.modsByID.excludeIDs = this.currentMods;
this.modsByID.moderatorsStatus.setState({
hidden: true,
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These three sections of code were removed, they were just duplicating things already handled by the view and creating some weird edge case behavior (like if you loaded moderators, then closed and reopened settings the status bar would appear).


this.modsSelected.moderatorsCol.add([...remByID, ...remAvail]);
this.modsSelected.moderatorsStatus.setState({
hidden: true,
});

this.modsAvailable.excludeIDs = this.currentMods;
this.modsAvailable.moderatorsCol.add(remSel);
this.modsAvailable.moderatorsStatus.setState({
hidden: false,
total: this.modsAvailable.modCount,
showSpinner: false,
});

// If any of the mods moved to the available collect are unverified, show them
if (app.verifiedMods.matched(unSel).length !== unSel.length) {
Expand Down
2 changes: 1 addition & 1 deletion js/views/modals/purchase/Purchase.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ export default class extends BaseModal {
singleSelect: true,
radioStyle: true,
initialState: {
showOnlyCur: currencies[0],
showOnlyCur: '',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was only showing moderators that accept BTC, I assume it was there because originally we automatically selected the first currency. Since we changed it to require the user to select a currency (none are selected by default) this should also have no default.

showVerifiedOnly: true,
},
});
Expand Down