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

No longer redirect if ARK or DOI is only a partial match (except for the prefix, which is optional) #896

Merged
merged 6 commits into from
Jan 24, 2023

Conversation

mccalluc
Copy link
Contributor

I've implemented one exception as suggested in that issue: either ID can be provided without the prefix. I don't know how this feature is being used, so I'm not confident that is the right course.

The test is repetitive: I've tried making it more DRY... but that also makes it harder to read. Whatever the reviewer thinks best: Either approve the current state, or request changes and I could paste it in.

More DRY
    describe "ID redirection" do
      {
        doi: "10.34770/",
        ark: "ark:/"
      }.each do |method_sym, prefix|
        resolve_method_sym = "resolve_#{method_sym}"
        describe resolve_method_sym do
          before do
            sign_in user
            stub_s3 data: data
          end

          let(:data) { [] }
          let(:work) { FactoryBot.create(:shakespeare_and_company_work) }

          it "redirects to the Work show view" do
            stub_s3
            get resolve_method_sym, params: { method_sym => work.send(method_sym) }
            expect(response).to redirect_to(work_path(work))
          end

          context "when passing only a segment of the #{method_sym}" do
            it "redirects to the Work show view if missing prefix" do
              stub_s3
              expect(work.send(method_sym)).to start_with(prefix)
              get resolve_method_sym, params: { method_sym => work.send(method_sym).gsub(prefix, "") }
              expect(response).to redirect_to(work_path(work))
            end

            it "does not redirect to the Work show view if not exact (missing slash)" do
              stub_s3
              expect do
                get resolve_method_sym, params: { method_sym => work.send(method_sym).gsub(prefix[0...-1], "") }
              end.to raise_error(ActiveRecord::RecordNotFound)
            end
          end
        end
      end
    end

rspec spec/controllers/works_controller_spec.rb:805 -f d

WorksController
  valid user login
    ID redirection
      resolve_ark
        redirects to the Work show view
        when passing only a segment of the ark
          redirects to the Work show view if missing prefix
          does not redirect to the Work show view if not exact (missing slash)
      resolve_doi
        redirects to the Work show view
        when passing only a segment of the doi
          does not redirect to the Work show view if not exact (missing slash)
          redirects to the Work show view if missing prefix

@hectorcorrea hectorcorrea merged commit 93f2b7f into main Jan 24, 2023
@hectorcorrea hectorcorrea deleted the mccalluc-no-partial-match-ark-doi branch January 24, 2023 15:50
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.

We should not allow partial matches
2 participants