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

Fix PR #871 #879

Closed
wants to merge 1 commit into from
Closed

Fix PR #871 #879

wants to merge 1 commit into from

Conversation

ares
Copy link
Member

@ares ares commented Sep 12, 2013

This one fixes #871, leaving as separate commits to review, I can squash when ready. Also @domcleals's commit was rebased to develop, that's why it has another hash. Test should be green now. Please review also other fixes.

@@ -48,8 +49,10 @@ def self.[](name)
def self.[]=(name, value)
name = name.to_s
record = find_or_create_by_name name
record.value = value
record.save!
Setting.bypass_readonly(record) do |rec|
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this needed for anything? This is the one place we shouldn't really override readonly...

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not needed, I just wanted to make sure values are updated. First I thought about readonly settings only as a UI functionality but you're correct, with this there would be no sense in having bypass method. I reverted this.

@ares
Copy link
Member Author

ares commented Sep 13, 2013

I squashed to one commit, feel free to merge this instead of #871 or update your original PR.

@domcleal
Copy link
Contributor

👍 from me for @ares' changes.

@domcleal
Copy link
Contributor

Merged as 4dbf662, thanks for the help @ares!

@domcleal domcleal closed this Sep 17, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants