Skip to content
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.

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

Fetch more than one season at once #236

Merged
merged 1 commit into from Jan 14, 2017

Conversation

Elsensee
Copy link
Contributor

Fixes #131

Copy link
Member

@dularion dularion left a comment

Choose a reason for hiding this comment

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

Very good work! thank you so much!! I have very little feedback. Thank you for the contribution

@@ -115,7 +115,7 @@
<div ng-show="!showLoading">

<button class="btn btn-primary" ng-click="addNewEpisode()">Add New Episode Manually</button>
<button class="btn btn-primary" ng-click="fetchAllEpisodesForSeason()" ng-if="!show.manualInput">Fetch all Episode for Season</button>
<button class="btn btn-primary" ng-click="fetchEpisodes()" ng-if="!show.manualInput">Fetch Episodes</button>
Copy link
Member

Choose a reason for hiding this comment

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

did you adjust the existing buttons functionality? How exactly does it work? if the user does not choose a season number, it will fetch all?

});
};

$scope.fetchEpisodes = function(){
alertify.set({ buttonReverse: true, labels: {ok: "OK", cancel : "Cancel"}});
alertify.prompt("For which seasons would you like to fetch the episodes? (Leave blank to fetch for all seasons)", function (confirmed, season) {
Copy link
Member

Choose a reason for hiding this comment

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

maybe give user more clear instructions, for example that he can use 1-3 syntax.

});
return;
} else if (season.indexOf('-') > -1) {
var start = parseInt(season.substring(0, season.indexOf('-')));
Copy link
Member

Choose a reason for hiding this comment

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

maybe safer approach is to split by '-' and then use the resulting array pos 0 and 1, trim them, and then parse them. Consider if the user types "1 - 5"


var getEpisodesForSeasons = function (seasons) {
var promises = [];
for (var i = 0; i < seasons.length; i++) {
Copy link
Member

Choose a reason for hiding this comment

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

avoid for loops, use lodash _.forEach instead for legibility.

@dularion dularion merged commit 73c0825 into streamaserver:grails3 Jan 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants