-
-
Notifications
You must be signed in to change notification settings - Fork 480
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
Enable/ disable revert using only a settings attribute #632
Conversation
Codecov Report
@@ Coverage Diff @@
## master #632 +/- ##
========================================
Coverage ? 97.7%
========================================
Files ? 17
Lines ? 914
Branches ? 136
========================================
Hits ? 893
Misses ? 9
Partials ? 12
Continue to review full report at Codecov.
|
@rossmechanic, I've separated this functionality out of the larger PR (#631) as I think you had requested some time ago. The next step, if this is accepted, is to complete #631. The new setting attribute introduced here will be needed for #631. |
{% if not revert_disabled %}<input type="submit" value="{% trans 'Revert' %}" class="default" name="_save" {{ onclick_attrib }}/>{% endif %} | ||
{% if change_history %}<input type="submit" value="{% trans 'Change History' %}" class="default" name="_change_history" {{ onclick_attrib }}/>{% endif %} | ||
<a href="{{ history_url }}" class="closelink">{% trans 'Close' %}</a> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing a new line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
@@ -8,7 +8,7 @@ | |||
{% block content %} | |||
<div id="content-main"> | |||
|
|||
<p>{% blocktrans %}Choose a date from the list below to revert to a previous version of this object.{% endblocktrans %}</p> | |||
{% if revert_disabled %}<p>{% blocktrans %}Choose a date from the list below to revert to a previous version of this object.{% endblocktrans %}</p>{% endif %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be not revert_disabled
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, yes. you are correct. added the condition to tests as well. thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. I think I'd like to allow this on a model-level in the future, but this is great for now. Thanks @erikvw . Sorry I took awhile to get to this
thanks and agreed. using the admin permissions of the history objects,
which give you model level granularity, is the one way to go.
…On Thu, Apr 23, 2020 at 8:52 AM Ross Mechanic ***@***.***> wrote:
***@***.**** approved this pull request.
This looks good to me. I think I'd like to allow this on a model-level in
the future, but this is great for now. Thanks @erikvw
<https://github.com/erikvw> . Sorry I took awhile to get to this
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#632 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAD2TZH3DMAKOUYA54DDMQ3ROBBZRANCNFSM4KYEKF5Q>
.
|
Description
Some installations may not wish to allow users to revert original model instances via the history record. This PR adds a settings attribute
SIMPLE_HISTORY_REVERT_DISABLED
that if set toTrue
removes theRevert
button from the history form.Motivation and Context
Makes it possible to globally disable the ability to revert a model instance via the historical record.
Types of changes
Checklist:
make format
command to format my codeAUTHORS.rst
CHANGES.rst