Revert "AO3-4359 Revert mass import bookmarks" #2499
+744
−413
1 participant
spec/api/api_bookmarks_spec.rb
| + | ||
| + it "should return an error message if there is no title" do | ||
| + post "/api/v1/bookmarks", | ||
| + { archivist: @user.login, | ||
| + bookmarks: [ bookmark.merge(title: "") ] | ||
| + }.to_json, | ||
| + valid_headers | ||
| + assert_equal 200, response.status | ||
| + bookmark_response = JSON.parse(response.body, symbolize_names: true)[:bookmarks].first | ||
| + assert_equal bookmark_response[:messages].first, "Title can't be blank" | ||
| + end | ||
| + | ||
| + it "should return an error message if there is no author" do | ||
| + post "/api/v1/bookmarks", | ||
| + { archivist: @user.login, | ||
| + bookmarks: [ bookmark.merge(author: "") ] |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
spec/api/api_bookmarks_spec.rb
| + it "should return an error message if there is no fandom" do | ||
| + post "/api/v1/bookmarks", | ||
| + { archivist: @user.login, | ||
| + bookmarks: [ bookmark.merge(fandom_string: "") ] | ||
| + }.to_json, | ||
| + valid_headers | ||
| + assert_equal 200, response.status | ||
| + bookmark_response = JSON.parse(response.body, symbolize_names: true)[:bookmarks].first | ||
| + assert_equal bookmark_response[:messages].first, | ||
| + "This bookmark does not contain a fandom. Please specify a fandom." | ||
| + end | ||
| + | ||
| + it "should return an error message if there is no title" do | ||
| + post "/api/v1/bookmarks", | ||
| + { archivist: @user.login, | ||
| + bookmarks: [ bookmark.merge(title: "") ] |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
spec/api/api_bookmarks_spec.rb
| + after do | ||
| + WebMock.reset! | ||
| + end | ||
| + | ||
| + | ||
| + it "should return 400 Bad Request if no bookmarks are specified" do | ||
| + post "/api/v1/bookmarks", | ||
| + { archivist: @user.login }.to_json, | ||
| + valid_headers | ||
| + assert_equal 400, response.status | ||
| + end | ||
| + | ||
| + it "should return an error mess if no URL is specified" do | ||
| + post "/api/v1/bookmarks", | ||
| + { archivist: @user.login, | ||
| + bookmarks: [ bookmark.except(:url) ] |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
spec/api/api_bookmarks_spec.rb
| + post "/api/v1/bookmarks", | ||
| + { archivist: @user.login, | ||
| + bookmarks: [ bookmark.except(:url) ] | ||
| + }.to_json, | ||
| + valid_headers | ||
| + assert_equal 200, response.status | ||
| + bookmark_response = JSON.parse(response.body, symbolize_names: true)[:bookmarks].first | ||
| + assert_equal bookmark_response[:messages].first, | ||
| + "This bookmark does not contain a URL to an external site. Please specify a valid, non-AO3 URL." | ||
| + end | ||
| + | ||
| + it "should return an error message if the URL is on AO3" do | ||
| + work = create(:work) | ||
| + post "/api/v1/bookmarks", | ||
| + { archivist: @user.login, | ||
| + bookmarks: [ bookmark.merge(url: work_url(work)) ] |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
spec/api/api_bookmarks_spec.rb
| + work = create(:work) | ||
| + post "/api/v1/bookmarks", | ||
| + { archivist: @user.login, | ||
| + bookmarks: [ bookmark.merge(url: work_url(work)) ] | ||
| + }.to_json, | ||
| + valid_headers | ||
| + assert_equal 200, response.status | ||
| + bookmark_response = JSON.parse(response.body, symbolize_names: true)[:bookmarks].first | ||
| + assert_equal bookmark_response[:messages].first, | ||
| + "Url could not be reached. If the URL is correct and the site is currently down, please try again later." | ||
| + end | ||
| + | ||
| + it "should return an error message if there is no fandom" do | ||
| + post "/api/v1/bookmarks", | ||
| + { archivist: @user.login, | ||
| + bookmarks: [ bookmark.merge(fandom_string: "") ] |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
spec/api/api_bookmarks_spec.rb
| + it "should pass back any original references unchanged" do | ||
| + post "/api/v1/bookmarks/import", | ||
| + { archivist: @user.login, | ||
| + bookmarks: [ bookmark ] | ||
| + }.to_json, | ||
| + valid_headers | ||
| + bookmark_response = JSON.parse(response.body, symbolize_names: true)[:bookmarks].first | ||
| + assert_equal "123", bookmark_response[:original_id], "Original reference should be passed back unchanged" | ||
| + assert_equal "http://foo.com", bookmark_response[:original_url], "Original URL should be passed back unchanged" | ||
| + end | ||
| + | ||
| + it "should respond with the URL of the created bookmark" do | ||
| + pseud_id = @user.default_pseud.id | ||
| + post "/api/v1/bookmarks/import", | ||
| + { archivist: @user.login, | ||
| + bookmarks: [ bookmark ] |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
spec/api/api_bookmarks_spec.rb
| + | ||
| + it "should not create duplicate bookmarks for the same archivist and external URL" do | ||
| + pseud_id = @user.default_pseud.id | ||
| + post "/api/v1/bookmarks/import", | ||
| + { archivist: @user.login, | ||
| + bookmarks: [ bookmark, bookmark ] | ||
| + }.to_json, | ||
| + valid_headers | ||
| + bookmarks = Bookmark.find_all_by_pseud_id(pseud_id) | ||
| + assert_equal bookmarks.count, 1 | ||
| + end | ||
| + | ||
| + it "should pass back any original references unchanged" do | ||
| + post "/api/v1/bookmarks/import", | ||
| + { archivist: @user.login, | ||
| + bookmarks: [ bookmark ] |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
spec/api/api_bookmarks_spec.rb
| + WebMock.reset! | ||
| + end | ||
| + | ||
| + it "should return 200 OK when all bookmarks are created" do | ||
| + post "/api/v1/bookmarks/import", | ||
| + { archivist: @user.login, | ||
| + bookmarks: [ bookmark ] | ||
| + }.to_json, | ||
| + valid_headers | ||
| + assert_equal 200, response.status | ||
| + end | ||
| + | ||
| + it "should return 200 OK when no bookmarks are created" do | ||
| + post "/api/v1/bookmarks/import", | ||
| + { archivist: @user.login, | ||
| + bookmarks: [ bookmark ] |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
spec/api/api_bookmarks_spec.rb
| + end | ||
| + | ||
| + it "should return 200 OK when no bookmarks are created" do | ||
| + post "/api/v1/bookmarks/import", | ||
| + { archivist: @user.login, | ||
| + bookmarks: [ bookmark ] | ||
| + }.to_json, | ||
| + valid_headers | ||
| + assert_equal 200, response.status | ||
| + end | ||
| + | ||
| + it "should not create duplicate bookmarks for the same archivist and external URL" do | ||
| + pseud_id = @user.default_pseud.id | ||
| + post "/api/v1/bookmarks/import", | ||
| + { archivist: @user.login, | ||
| + bookmarks: [ bookmark, bookmark ] |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
spec/api/api_bookmarks_spec.rb
| + rec: "0" } | ||
| + | ||
| + describe "Valid API bookmark import" do | ||
| + before do | ||
| + mock_external | ||
| + @user = create(:user) | ||
| + end | ||
| + | ||
| + after do | ||
| + WebMock.reset! | ||
| + end | ||
| + | ||
| + it "should return 200 OK when all bookmarks are created" do | ||
| + post "/api/v1/bookmarks/import", | ||
| + { archivist: @user.login, | ||
| + bookmarks: [ bookmark ] |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
spec/api/api_bookmarks_spec.rb
| + end | ||
| + | ||
| + WebMock.allow_net_connect! | ||
| + end | ||
| + | ||
| + describe "Invalid API bookmark import" do | ||
| + before do | ||
| + mock_external | ||
| + @user = create(:user) | ||
| + end | ||
| + | ||
| + after do | ||
| + WebMock.reset! | ||
| + end | ||
| + | ||
| + |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
app/controllers/api/v1/works_controller.rb
| private | ||
| + # Set messages based on success and error flags | ||
| + def response_message(messages) | ||
| + if @some_success && @some_errors |
|
Use the return of the conditional for variable assignment and comparison.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
app/controllers/api/v1/base_controller.rb
| + # if no works are supplied. If there is neither a valid archivist nor valid works, a 400 is returned | ||
| + # with both errors as a message | ||
| + def batch_errors(archivist, import_items) | ||
| + status = :bad_request | ||
| + errors = [] | ||
| + | ||
| + unless archivist && archivist.is_archivist? | ||
| + status = :forbidden | ||
| + errors << "The 'archivist' field must specify the name of an Archive user with archivist privileges." | ||
| + end | ||
| + | ||
| + if import_items.nil? || import_items.empty? | ||
| + errors << "No items to import were provided." | ||
| + elsif import_items.size >= ArchiveConfig.IMPORT_MAX_WORKS_BY_ARCHIVIST | ||
| + errors << "This request contains too many items to import. A maximum of #{ ArchiveConfig.IMPORT_MAX_WORKS_BY_ARCHIVIST }" + | ||
| + "items can be imported at one time by an archivist." |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
app/controllers/api/v1/bookmarks_controller.rb
| + bookmarks.each do |bookmark| | ||
| + bookmarks_responses << import_bookmark(archivist, bookmark) | ||
| + end | ||
| + | ||
| + # set final response code and message depending on the flags | ||
| + messages = response_message(messages) | ||
| + end | ||
| + | ||
| + render status: status, json: { status: status, messages: messages, bookmarks: bookmarks_responses } | ||
| + end | ||
| + | ||
| + private | ||
| + | ||
| + # Set messages based on success and error flags | ||
| + def response_message(messages) | ||
| + if @some_success && @some_errors |
|
Use the return of the conditional for variable assignment and comparison.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
app/controllers/api/v1/base_controller.rb
| + # Top-level error handling: returns a 403 forbidden if a valid archivist isn't supplied and a 400 | ||
| + # if no works are supplied. If there is neither a valid archivist nor valid works, a 400 is returned | ||
| + # with both errors as a message | ||
| + def batch_errors(archivist, import_items) | ||
| + status = :bad_request | ||
| + errors = [] | ||
| + | ||
| + unless archivist && archivist.is_archivist? | ||
| + status = :forbidden | ||
| + errors << "The 'archivist' field must specify the name of an Archive user with archivist privileges." | ||
| + end | ||
| + | ||
| + if import_items.nil? || import_items.empty? | ||
| + errors << "No items to import were provided." | ||
| + elsif import_items.size >= ArchiveConfig.IMPORT_MAX_WORKS_BY_ARCHIVIST | ||
| + errors << "This request contains too many items to import. A maximum of #{ ArchiveConfig.IMPORT_MAX_WORKS_BY_ARCHIVIST }" + |
|
Use
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reverts otwcode/otwarchive#2498