Code review comment for lp:~ce-infrastructure/capomastro/capomastro-jenkins-exposure-1155

Revision history for this message
Caio Begotti (caio1982) wrote :

Thanks for the Django tips, I didn't know how we could tests a template but
I'll take a look at it and use a small template just for this alert then.

On Thu, Nov 20, 2014 at 8:23 PM, Sheila Miguez <email address hidden>
wrote:

> I added a diff command about using "include" as an option.
>
> Diff comments:
>
> > === modified file 'capomastro/local_settings.py'
> > --- capomastro/local_settings.py 2014-11-03 19:06:32 +0000
> > +++ capomastro/local_settings.py 2014-11-20 20:20:15 +0000
> > @@ -1,6 +1,5 @@
> > # SECURITY WARNING: don't run with debug turned on in production!
> > -DEBUG = False
> > -
> > +DEBUG = True
> > TEMPLATE_DEBUG = True
> > ALLOWED_HOSTS = []
> >
> >
> > === modified file 'debian/changelog'
> > --- debian/changelog 2014-11-03 19:06:32 +0000
> > +++ debian/changelog 2014-11-20 20:20:15 +0000
> > @@ -1,3 +1,10 @@
> > +capomastro (20141120) trusty; urgency=high
> > +
> > + * Adds warning to avoid overexposing the Jenkins server.
> > + * Adds two testing scripts to use with LXC and Vagrant.
> > +
> > + -- Caio Begotti <email address hidden> Thu, 20 Nov 2014
> 10:41:23 -0200
> > +
> > capomastro (20141103) trusty; urgency=high
> >
> > * Partially fixes https://bugs.launchpad.net/capomastro/+bug/1388936
> >
> > === modified file 'projects/templates/projects/dependency_detail.html'
> > --- projects/templates/projects/dependency_detail.html 2014-07-18
> 12:07:38 +0000
> > +++ projects/templates/projects/dependency_detail.html 2014-11-20
> 20:20:15 +0000
> > @@ -11,6 +11,9 @@
> > <div class="container">
> > <div class="row">
> > <div>
> > + {% if not archiver %}
> > + {% bootstrap_alert "Could not find a default archiver in the
> configuration. <a href='/admin/archives/archive/'>Please set up one</a>,
> otherwise artifacts of this dependency will only be available through an
> exposed Jenkins server." alert_type='warning' %}
> > + {% endif %}
>
> One way to handle it is to use an include.
>
> {% include "archiver_alert.html" %}
>
> you can also pass objects to it in case you want to tweak the snippet, for
> example
>
> {% include "archiver_alert.html" with somevar="foobar"%}
>
> > <h2>{{ dependency.name }}</h2>
> > {% if dependency.get_build_parameters %}
> > <table class="table table-striped">
> >
> > === modified file 'projects/templates/projects/dependency_form.html'
> > --- projects/templates/projects/dependency_form.html 2014-06-02
> 10:55:15 +0000
> > +++ projects/templates/projects/dependency_form.html 2014-11-20
> 20:20:15 +0000
> > @@ -8,6 +8,9 @@
> > <div class="container">
> > <div class="row">
> > <div>
> > + {% if not archiver %}
> > + {% bootstrap_alert "Could not find a default archiver in the
> configuration. <a href='/admin/archives/archive/'>Please set up one</a>,
> otherwise artifacts of this dependency will only be available through an
> exposed Jenkins server." alert_type='warning' %}
> > + {% endif %}
> > <h2>Dependency</h2>
> > <form action="" method="post" class="form" id="dependency">
> > {% csrf_token %}
> >
> > === modified file 'projects/templates/projects/project_detail.html'
> > --- projects/templates/projects/project_detail.html 2014-07-25
> 11:37:42 +0000
> > +++ projects/templates/projects/project_detail.html 2014-11-20
> 20:20:15 +0000
> > @@ -8,6 +8,9 @@
> > <div class="container">
> > <div class="row">
> > <div>
> > + {% if not archiver %}
> > + {% bootstrap_alert "Could not find a default archiver in the
> configuration. <a href='/admin/archives/archive/'>Please set up one</a>,
> otherwise artifacts of this project will only be available through an
> exposed Jenkins server." alert_type='warning' %}
> > + {% endif %}
> > <h2>{{ project.name }}</h2>
> > </div>
> > </div>
> >
> > === modified file 'projects/templates/projects/project_form.html'
> > --- projects/templates/projects/project_form.html 2014-03-20
> 16:17:58 +0000
> > +++ projects/templates/projects/project_form.html 2014-11-20
> 20:20:15 +0000
> > @@ -8,6 +8,9 @@
> > <div class="container">
> > <div class="row">
> > <div>
> > + {% if not archiver %}
> > + {% bootstrap_alert "Could not find a default archiver in the
> configuration. <a href='/admin/archives/archive/'>Please set up one</a>,
> otherwise artifacts of this project will only be available through an
> exposed Jenkins server." alert_type='warning' %}
> > + {% endif %}
> > <h2>Project</h2>
> > <form action="" method="post" class="form" id="project">
> > {% csrf_token %}
> >
> > === modified file 'projects/templates/projects/projectbuild_detail.html'
> > --- projects/templates/projects/projectbuild_detail.html 2014-07-25
> 11:37:42 +0000
> > +++ projects/templates/projects/projectbuild_detail.html 2014-11-20
> 20:20:15 +0000
> > @@ -31,7 +31,7 @@
> > <td>{{ dependency.build.number }}</td>
> > <td>{{ dependency.dependency.name }}</td>
> > <td>{{ dependency.build.status }}</td>
> > - <td><a href="{{ dependency.build.url }}">{{
> dependency.build.phase }}</a></td>
> > + <td>{{ dependency.build.phase }}</td>
> > {% else %}
> > <td colspan="4">Waiting for build of job {{ dependency.job
> }}</td>
> > {% endif %}
> >
> > === modified file 'projects/views.py'
> > --- projects/views.py 2014-07-25 14:58:58 +0000
> > +++ projects/views.py 2014-11-20 20:20:15 +0000
> > @@ -46,6 +46,19 @@
> > initial["auto_track"] = True
> > return initial
> >
> > + def get_context_data(self, **kwargs):
> > + """
> > + Supplement the project view with an archive check
> > + """
> > + context = super(
> > + ProjectCreateView, self).get_context_data(**kwargs)
> > +
> > + archive = get_default_archive()
> > + if archive:
> > + context["archiver"] = archive
> > +
> > + return context
> > +
> >
> > class ProjectListView(LoginRequiredMixin, ListView):
> >
> > @@ -194,6 +207,11 @@
> > for artifact in
> current_projectbuild.get_current_artifacts():
> > items.append(self.item_from_artifact(artifact))
> > context["current_artifacts"] = items
> > +
> > + archive = get_default_archive()
> > + if archive:
> > + context["archiver"] = archive
> > +
> > return context
> >
> > @staticmethod
> > @@ -237,6 +255,20 @@
> > url_args = {"pk": self.object.pk}
> > return reverse("dependency_detail", kwargs=url_args)
> >
> > + def get_context_data(self, **kwargs):
> > + """
> > + Supplement the dependency view with an archive check
> > + """
> > + context = super(
> > + DependencyCreateView, self).get_context_data(**kwargs)
> > +
> > + archive = get_default_archive()
> > + if archive:
> > + context["archiver"] = archive
> > +
> > + return context
> > +
> > +
> >
> > class DependencyListView(LoginRequiredMixin, ListView):
> >
> > @@ -285,6 +317,11 @@
> > messages.add_message(
> > self.request, messages.INFO,
> > "Dependency currently building")
> > +
> > + archive = get_default_archive()
> > + if archive:
> > + context["archiver"] = archive
> > +
> > return context
> >
> > def post(self, request, pk):
> >
>
>
> --
>
> https://code.launchpad.net/~ce-infrastructure/capomastro/capomastro-jenkins-exposure-1155/+merge/242412
> Your team CE Infrastructure is subscribed to branch lp:capomastro.
>

« Back to merge proposal