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.

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

Jump to bottom

Concept for simplifying get & watch in builds controller #133

Merged
merged 1 commit into from
Jul 7, 2016
Merged

Concept for simplifying get & watch in builds controller #133

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
125 changes: 45 additions & 80 deletions app/scripts/controllers/build.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@ angular.module('openshiftConsole')
$scope.selectedTab[$routeParams.tab] = true;
}

var buildConfigForBuild = $filter('buildConfigForBuild');

var watches = [];

var setLogVars = function(build) {
Expand All @@ -59,6 +57,44 @@ angular.module('openshiftConsole')
}
};

var buildResolved = function(build, action) {
$scope.loaded = true;
$scope.build = build;
setLogVars(build);
var buildNumber = $filter("annotation")(build, "buildNumber");
if (buildNumber) {
$scope.breadcrumbs[2].title = "#" + buildNumber;
}
if (action === "DELETED") {
$scope.alerts["deleted"] = {
type: "warning",
message: "This build has been deleted."
};
}
};

var buildRejected = function(e) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The above eliminates the most duplication. I pulled the rest of these out for consistency only.

$scope.loaded = true;
$scope.alerts["load"] = {
type: "error",
message: "The build details could not be loaded.",
details: "Reason: " + $filter('getErrorDetails')(e)
};
};

var buildConfigResolved = function(buildConfig, action) {
if (action === "DELETED") {
$scope.alerts["deleted"] = {
type: "warning",
message: "Build configuration " + $scope.buildConfigName + " has been deleted."
};
}
$scope.buildConfig = buildConfig;
$scope.paused = BuildsService.isPaused($scope.buildConfig);
updateCanBuild();
};


ProjectsService
.get($routeParams.project)
.then(_.spread(function(project, context) {
Expand All @@ -69,84 +105,13 @@ angular.module('openshiftConsole')
// context into the log-viewer directive.
$scope.projectContext = context;
$scope.logOptions = {};
DataService.get("builds", $routeParams.build, context).then(
// success
function(build) {

$scope.loaded = true;
$scope.build = build;
setLogVars(build);
var buildNumber = $filter("annotation")(build, "buildNumber");
if (buildNumber) {
$scope.breadcrumbs[2].title = "#" + buildNumber;
}

// If we found the item successfully, watch for changes on it
watches.push(DataService.watchObject("builds", $routeParams.build, context, function(build, action) {
if (action === "DELETED") {
$scope.alerts["deleted"] = {
type: "warning",
message: "This build has been deleted."
};
}
$scope.build = build;
setLogVars(build);
}));
watches.push(DataService.watchObject("buildconfigs", $routeParams.buildconfig, context, function(buildConfig, action) {
if (action === "DELETED") {
$scope.alerts["deleted"] = {
type: "warning",
message: "Build configuration " + $scope.buildConfigName + " has been deleted."
};
}
$scope.buildConfig = buildConfig;
$scope.paused = BuildsService.isPaused($scope.buildConfig);
updateCanBuild();
}));
},
// failure
function(e) {
$scope.loaded = true;
$scope.alerts["load"] = {
type: "error",
message: "The build details could not be loaded.",
details: "Reason: " + $filter('getErrorDetails')(e)
};
}
);

watches.push(DataService.watch("builds", context, function(builds, action, build) {
if (!action) {
$scope.builds = BuildsService.validatedBuildsForBuildConfig($routeParams.buildconfig, builds.by('metadata.name'));
} else {
var buildConfigName = buildConfigForBuild(build);
if (buildConfigName === $routeParams.buildconfig) {
var buildName = build.metadata.name;
switch (action) {
case 'ADDED':
case 'MODIFIED':
$scope.builds[buildName] = build;
break;
case 'DELETED':
delete $scope.builds[buildName];
break;
}
}
}

updateCanBuild();
},
// params object for filtering
{
// http is passed to underlying $http calls
http: {
params: {
// because build config names can be > 63 chars but label values can't
// and we can't do a fieldSelector on annotations. Plus old builds dont have the annotation.
labelSelector: $filter('labelName')('buildConfig') + '=' + _.trunc($routeParams.buildconfig, {length: 63, omission: ''})
}
}
}));
DataService
.get("builds", $routeParams.build, context)
.then(function(build) {
buildResolved(build);
watches.push(DataService.watchObject("builds", $routeParams.build, context, buildResolved));
Copy link
Member

Choose a reason for hiding this comment

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

now that we've plumbed the labelSelector / fieldSelector stuff, we should probably update the watchObject in DataService to tack on the fieldSelector for the name of the object so we aren't pulling a bunch of data we don't need, doesn't need to be done in this PR, just thinking out loud :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah definitely!

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'll create an issue for tracking that.

watches.push(DataService.watchObject("buildconfigs", $routeParams.buildconfig, context, buildConfigResolved));
}, buildRejected);

$scope.toggleSecret = function() {
$scope.showSecret = true;
Expand Down
68 changes: 21 additions & 47 deletions dist/scripts/scripts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3482,67 +3482,41 @@ link:"project/" + b.project + "/browse/builds/" + b.buildconfig
}), a.breadcrumbs.push({
title:b.build
}), b.tab && (a.selectedTab = {}, a.selectedTab[b.tab] = !0);
var g = f("buildConfigForBuild"), h = [], i = function(b) {
var g = [], h = function(b) {
a.logOptions.container = f("annotation")(b, "buildPod"), a.logCanRun = !_.includes([ "New", "Pending", "Error" ], b.status.phase);
}, j = function() {
}, i = function() {
a.buildConfig ? a.canBuild = e.canBuild(a.buildConfig) :a.canBuild = !1;
};
d.get(b.project).then(_.spread(function(d, k) {
a.project = d, a.projectContext = k, a.logOptions = {}, c.get("builds", b.build, k).then(function(d) {
a.loaded = !0, a.build = d, i(d);
var g = f("annotation")(d, "buildNumber");
g && (a.breadcrumbs[2].title = "#" + g), h.push(c.watchObject("builds", b.build, k, function(b, c) {
"DELETED" === c && (a.alerts.deleted = {
}, j = function(b, c) {
a.loaded = !0, a.build = b, h(b);
var d = f("annotation")(b, "buildNumber");
d && (a.breadcrumbs[2].title = "#" + d), "DELETED" === c && (a.alerts.deleted = {
type:"warning",
message:"This build has been deleted."
}), a.build = b, i(b);
})), h.push(c.watchObject("buildconfigs", b.buildconfig, k, function(b, c) {
"DELETED" === c && (a.alerts.deleted = {
type:"warning",
message:"Build configuration " + a.buildConfigName + " has been deleted."
}), a.buildConfig = b, a.paused = e.isPaused(a.buildConfig), j();
}));
}, function(b) {
});
}, k = function(b) {
a.loaded = !0, a.alerts.load = {
type:"error",
message:"The build details could not be loaded.",
details:"Reason: " + f("getErrorDetails")(b)
};
}), h.push(c.watch("builds", k, function(c, d, f) {
if (d) {
var h = g(f);
if (h === b.buildconfig) {
var i = f.metadata.name;
switch (d) {
case "ADDED":
case "MODIFIED":
a.builds[i] = f;
break;

case "DELETED":
delete a.builds[i];
}
}
} else a.builds = e.validatedBuildsForBuildConfig(b.buildconfig, c.by("metadata.name"));
j();
}, {
http:{
params:{
labelSelector:f("labelName")("buildConfig") + "=" + _.trunc(b.buildconfig, {
length:63,
omission:""
})
}
}
})), a.toggleSecret = function() {
}, l = function(b, c) {
"DELETED" === c && (a.alerts.deleted = {
type:"warning",
message:"Build configuration " + a.buildConfigName + " has been deleted."
}), a.buildConfig = b, a.paused = e.isPaused(a.buildConfig), i();
};
d.get(b.project).then(_.spread(function(d, f) {
a.project = d, a.projectContext = f, a.logOptions = {}, c.get("builds", b.build, f).then(function(a) {
j(a), g.push(c.watchObject("builds", b.build, f, j)), g.push(c.watchObject("buildconfigs", b.buildconfig, f, l));
}, k), a.toggleSecret = function() {
a.showSecret = !0;
}, a.cancelBuild = function() {
e.cancelBuild(a.build, a.buildConfigName, k, a);
e.cancelBuild(a.build, a.buildConfigName, f, a);
}, a.cloneBuild = function() {
var b = _.get(a, "build.metadata.name");
b && a.canBuild && e.cloneBuild(b, k, a);
b && a.canBuild && e.cloneBuild(b, f, a);
}, a.$on("$destroy", function() {
c.unwatchAll(h);
c.unwatchAll(g);
});
}));
} ]), angular.module("openshiftConsole").controller("ImagesController", [ "$routeParams", "$scope", "AlertMessageService", "DataService", "ProjectsService", "$filter", "LabelFilter", "Logger", function(a, b, c, d, e, f, g, h) {
Expand Down