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

Give SINGLE_USER a chance to register #1820

Merged
merged 1 commit into from Apr 15, 2017
Merged

Conversation

saper
Copy link
Contributor

@saper saper commented Apr 15, 2017

An attempt to open a brand new Mastodon instance configured
as SINGLE_USER_MODE=true will cause an exception.

Enable temporary registration if we have no users in the database

Fixes #1817

@saper
Copy link
Contributor Author

saper commented Apr 15, 2017

Looks like controller helper_method is a trick that the test suite does not like, will have a look at it

@mjankowski
Copy link
Contributor

Instead of doing this check on every request, could we improve the docs to make it more clear that single user instances need to create accounts prior to being able to use the app?

@@ -69,6 +69,10 @@ def unprocessable_entity
end
end

def single_user_mode
Copy link
Member

Choose a reason for hiding this comment

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

I suggest renaming this to single_user_mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was that in my first draft :) thanks

@@ -69,6 +69,10 @@ def unprocessable_entity
end
end

def single_user_mode
Rails.configuration.x.single_user_mode && Account.first
Copy link
Member

Choose a reason for hiding this comment

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

And this could be @single_user_mode ||= Rails.configuration.x.single_user_mode && Account.first for multiple calls

Copy link
Member

@Gargron Gargron left a comment

Choose a reason for hiding this comment

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

Didn't mean to click approve, meant to click reqest changes

@saper
Copy link
Contributor Author

saper commented Apr 15, 2017

I was thinking about spitting out the error message instead.

Is the Account.first check expensive? If yes, then we could store the result somewhere?

@Gargron
Copy link
Member

Gargron commented Apr 15, 2017

@mjankowski The DB check only occurs when single user mode is enabled, and it would occur anyway for the redirect to the right profile, so I think that's not a big deal

An attempt to open a brand new Mastodon instance configured
as SINGLE_USER_MODE=true will cause an exception.

Enable temporary registration if we have no users in the database

Fixes mastodon#1817
@Gargron Gargron merged commit 1c8477e into mastodon:master Apr 15, 2017
@saper
Copy link
Contributor Author

saper commented Apr 15, 2017

Wow, thanks.

I've tried to fix the tests by doing

diff --git a/spec/views/accounts/show.html.haml_spec.rb b/spec/views/accounts/show.html.haml_spec.rb
index 8265b2f4..1fc75da7 100644
--- a/spec/views/accounts/show.html.haml_spec.rb
+++ b/spec/views/accounts/show.html.haml_spec.rb
@@ -10,6 +10,8 @@ describe "stream_entries/show.html.haml" do
   end
 
   it "has valid author h-card and basic data for a detailed_status" do
+    allow(view).to receive(:single_user_mode?).and_return(false)
+
     alice  =  Fabricate(:account, username: 'alice', display_name: 'Alice')
     bob    =  Fabricate(:account, username: 'bob', display_name: 'Bob')
     status =  Fabricate(:status, account: alice, text: 'Hello World')
@@ -32,6 +34,8 @@ describe "stream_entries/show.html.haml" do
   end
 
   it "has valid h-cites for p-in-reply-to and p-comment" do
+    allow(view).to receive(:single_user_mode?).and_return(false)
+
     alice   =  Fabricate(:account, username: 'alice', display_name: 'Alice')
     bob     =  Fabricate(:account, username: 'bob', display_name: 'Bob')
     carl    =  Fabricate(:account, username: 'carl', display_name: 'Carl')

but causes the <someverylangeclassdescription does not implement: single_user_mode? which looks like lack of the controller spec or rspec/rspec-rails#1076 to me

@Gargron
Copy link
Member

Gargron commented Apr 15, 2017

@saper Right, I am still encountering the test issue. Really annoying. Any solution?

@Gargron
Copy link
Member

Gargron commented Apr 15, 2017

Never mind, found a hacky way to fix those tests.

@saper
Copy link
Contributor Author

saper commented Apr 15, 2017

Nice.

What would be the proper way ? Providing spec for application controller?

Nyoho pushed a commit to Nyoho/mathtodon that referenced this pull request Apr 25, 2017
An attempt to open a brand new Mastodon instance configured
as SINGLE_USER_MODE=true will cause an exception.

Enable temporary registration if we have no users in the database

Fixes mastodon#1817
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants