Skip to content
This repository has been archived by the owner on Jan 14, 2021. It is now read-only.

Toggle monitoring button #96

Merged
merged 3 commits into from Oct 11, 2014
Merged

Toggle monitoring button #96

merged 3 commits into from Oct 11, 2014

Conversation

trasher
Copy link
Contributor

@trasher trasher commented Oct 10, 2014

No description provided.

@ralphbean
Copy link
Contributor

@trasher awesome :)

Can I ask for two things: Can we get a screenshot if that's easy to do? Can you also add a little paragraph of text or a link to https://fedoraproject.org/wiki/Upstream_Release_Monitoring next to the monitoring button so people know what it means? (Probably just the link will do)

@trasher
Copy link
Contributor Author

trasher commented Oct 10, 2014

I've added the link, met me know if it is ok for you :)
Also, here is a screenshot of that button:
pkgdb2_monitoring_btn

@ralphbean
Copy link
Contributor

I've added the link, met me know if it is ok for you :)

Yeah! Perfect!

Also, here is a screenshot of that button

The screenshot looks awesome. This get a 👍 to merge this from me but I'll wait for @pypingou to do the deed. :)

@trasher
Copy link
Contributor Author

trasher commented Oct 11, 2014

Actually, when user is not logged in, the button is still active; it should not.
Here's a patch to disable the button for non admins users:

 % git diff
diff --git a/pkgdb2/templates/package.html b/pkgdb2/templates/package.html
index 4e9fdfc..52400b3 100644
--- a/pkgdb2/templates/package.html
+++ b/pkgdb2/templates/package.html
@@ -28,9 +28,13 @@ confusing what the difference is between that status an the per-branch status)
         <th><a target="_blank" href="https://fedoraproject.org/wiki/Upstream_release_monitoring">Monitoring</a></th>
         <td>
             <div class="switch">
-                <input type="radio" class="switch-input" name="monitor" value="on" id="monitoron"{%if package.monitor %} checked="checked"{% endif %}>
+                <input type="radio" class="switch-input" name="monitor" value="on" id="monitoron"
+                    {%if package.monitor %} checked="checked"{% endif %}
+                    {% if not is_admin %} disabled="disabled"{% endif %}>
                 <label for="monitoron" class="switch-label switch-label-off">On</label>
-                <input type="radio" class="switch-input" name="monitor" value="off" id="monitoroff"{%if not package.monitor %} checked="checked"{% endif %}>
+                <input type="radio" class="switch-input" name="monitor" value="off" id="monitoroff"
+                    {%if not package.monitor %} checked="checked"{% endif %}
+                    {% if not is_admin %} disabled="disabled"{% endif %}>
                 <label for="monitoroff" class="switch-label switch-label-on">Off</label>
                 <span class="switch-selection"></span>
             </div>

I guess the if not is_admin is too much restrictive, maybe something like if not(g.fas_user.username in admins or is_admin) would be better. What do you think?

@pypingou
Copy link
Member

I'd let any of the contributors (w/ commit or approveacls) activate the switch.

Do you want to do the patch or shall I?

@trasher
Copy link
Contributor Author

trasher commented Oct 11, 2014

I've done the change, hope that is ok :)

@pypingou
Copy link
Member

Looks good, thanks!

pypingou added a commit that referenced this pull request Oct 11, 2014
@pypingou
Merge pull request #96 from trasher/monitoring
ff551bb 
Toggle monitoring button
@pypingou pypingou merged commit ff551bb into fedora-infra:monitoring Oct 11, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@trasher @ralphbean @pypingou