Fixes featured hashtag setting page erroring out instead of rejecting invalid tags (#12436)
* Revert "Fix ignoring whole status because of one invalid hashtag (#11621)"
This reverts commit dff46b260b.
* Fix statuses being rejected because of invalid hashtag names
* Add spec for invalid hashtag names in statuses
* Add test for featured tags controller
	
	
This commit is contained in:
		
					parent
					
						
							
								3830c0b741
							
						
					
				
			
			
				commit
				
					
						da2143b308
					
				
			
		
					 4 changed files with 67 additions and 2 deletions
				
			
		|  | @ -157,7 +157,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity | ||||||
|     return if tag['name'].blank? |     return if tag['name'].blank? | ||||||
| 
 | 
 | ||||||
|     Tag.find_or_create_by_names(tag['name']) do |hashtag| |     Tag.find_or_create_by_names(tag['name']) do |hashtag| | ||||||
|       @tags << hashtag unless @tags.include?(hashtag) |       @tags << hashtag unless @tags.include?(hashtag) || !hashtag.valid? | ||||||
|     end |     end | ||||||
|   rescue ActiveRecord::RecordInvalid |   rescue ActiveRecord::RecordInvalid | ||||||
|     nil |     nil | ||||||
|  |  | ||||||
|  | @ -117,7 +117,7 @@ class Tag < ApplicationRecord | ||||||
|   class << self |   class << self | ||||||
|     def find_or_create_by_names(name_or_names) |     def find_or_create_by_names(name_or_names) | ||||||
|       Array(name_or_names).map(&method(:normalize)).uniq { |str| str.mb_chars.downcase.to_s }.map do |normalized_name| |       Array(name_or_names).map(&method(:normalize)).uniq { |str| str.mb_chars.downcase.to_s }.map do |normalized_name| | ||||||
|         tag = matching_name(normalized_name).first || create!(name: normalized_name) |         tag = matching_name(normalized_name).first || create(name: normalized_name) | ||||||
| 
 | 
 | ||||||
|         yield tag if block_given? |         yield tag if block_given? | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
							
								
								
									
										43
									
								
								spec/controllers/settings/featured_tags_controller_spec.rb
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										43
									
								
								spec/controllers/settings/featured_tags_controller_spec.rb
									
										
									
									
									
										Normal file
									
								
							|  | @ -0,0 +1,43 @@ | ||||||
|  | require 'rails_helper' | ||||||
|  | 
 | ||||||
|  | describe Settings::FeaturedTagsController do | ||||||
|  |   render_views | ||||||
|  | 
 | ||||||
|  |   shared_examples 'authenticate user' do | ||||||
|  |     it 'redirects to sign_in page' do | ||||||
|  |       is_expected.to redirect_to new_user_session_path | ||||||
|  |     end | ||||||
|  |   end | ||||||
|  | 
 | ||||||
|  |   describe 'POST #create' do | ||||||
|  |     context 'when user is not sign in' do | ||||||
|  |       subject { post :create } | ||||||
|  | 
 | ||||||
|  |       it_behaves_like 'authenticate user' | ||||||
|  |     end | ||||||
|  | 
 | ||||||
|  |     context 'when user is sign in' do | ||||||
|  |       subject { post :create, params: { featured_tag: params } } | ||||||
|  | 
 | ||||||
|  |       let(:user) { Fabricate(:user, password: '12345678') } | ||||||
|  | 
 | ||||||
|  |       before { sign_in user, scope: :user } | ||||||
|  | 
 | ||||||
|  |       context 'when parameter is valid' do | ||||||
|  |         let(:params) { { name: 'test' } } | ||||||
|  | 
 | ||||||
|  |         it 'creates featured tag' do | ||||||
|  |           expect { subject }.to change { user.account.featured_tags.count }.by(1) | ||||||
|  |         end | ||||||
|  |       end | ||||||
|  | 
 | ||||||
|  |       context 'when parameter is invalid' do | ||||||
|  |         let(:params) { { name: 'test, #foo !bleh' } } | ||||||
|  | 
 | ||||||
|  |         it 'renders new' do | ||||||
|  |           expect(subject).to render_template :index | ||||||
|  |         end | ||||||
|  |       end | ||||||
|  |     end | ||||||
|  |   end | ||||||
|  | end | ||||||
|  | @ -378,6 +378,28 @@ RSpec.describe ActivityPub::Activity::Create do | ||||||
|         end |         end | ||||||
|       end |       end | ||||||
| 
 | 
 | ||||||
|  |       context 'with hashtags invalid name' do | ||||||
|  |         let(:object_json) do | ||||||
|  |           { | ||||||
|  |             id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, | ||||||
|  |             type: 'Note', | ||||||
|  |             content: 'Lorem ipsum', | ||||||
|  |             tag: [ | ||||||
|  |               { | ||||||
|  |                 type: 'Hashtag', | ||||||
|  |                 href: 'http://example.com/blah', | ||||||
|  |                 name: 'foo, #eh !', | ||||||
|  |               }, | ||||||
|  |             ], | ||||||
|  |           } | ||||||
|  |         end | ||||||
|  | 
 | ||||||
|  |         it 'creates status' do | ||||||
|  |           status = sender.statuses.first | ||||||
|  |           expect(status).to_not be_nil | ||||||
|  |         end | ||||||
|  |       end | ||||||
|  | 
 | ||||||
|       context 'with emojis' do |       context 'with emojis' do | ||||||
|         let(:object_json) do |         let(:object_json) do | ||||||
|           { |           { | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue