Skip to content

Commit 939fa85

Browse files
committed
Fix updating project feature spec
Previously this spec was not representative of how the API was being used in practice and it was only really working by accident. The PUT requests in the examples were using the `Project#id` rather than the `Project#identifier` as they should have been. The spec was working even though the `Api::ProjectsController#load_project` before action was *not* finding the project, because the `load_and_authorize_resource` before action was then finding the project using `Project.find(params[:id])` and setting the `@project` instance variable used by the rest of the logic in the controller. However, clients of editor-api like editor-standalone use the project identifier as the resource ID in the URL path [1], so this spec was completely unrepresentative of how the endpoint was really being used. In this commit I've changed the examples to use the `Project#identifier` as the resource ID in the PUT requests. This means that the `Api::ProjectsController#load_project` before action now finds the project by identifier and since it sets the `@project` instance variable the `load_and_authorize_resource` before action no longer attempts to load the resource, it just does the authorize step using the already loaded project. In order to make this work reliably, I had to explicitly set the `Project#locale` to one of the fallback locales in `ProjectLoader` [2], i.e. 'en' or `nil`. Otherwise, the random locale selected in the project factory [3] meant that sometimes the `load_project` before action (which uses the `ProjectLoader` did not find the project and the `load_and_authorize_resource` before action raised an `ActiveRecord::RecordNotFound` resulting in a 404 Not Found. Now the spec is no longer making use of the randome locale from the factory, I've taken the opportunity to add some new examples demonstrating the behaviour when the project has different locales. This effectively demonstrates that `load_project` is wired up to `ProjectLoader` correctly. As an aside, I'm not convinced that having the factory select a locale at random is a good idea. I found it very confusing when it led to undeterministic specs. However, I'll leave that for another time. [1]: https://github.com/RaspberryPiFoundation/editor-standalone/blob/1d4375635cb6890794732072d608dbd4b05b3bb0/src/utils/apiCallHandler/projects.js#L16 [2]: https://github.com/RaspberryPiFoundation/editor-api/blob/b4bd337d09a88b1f41ecdc13136f7d11da0dcf89/lib/project_loader.rb#L8 [3]: https://github.com/RaspberryPiFoundation/editor-api/blob/b4bd337d09a88b1f41ecdc13136f7d11da0dcf89/spec/factories/project.rb#L9
1 parent f874604 commit 939fa85

File tree

1 file changed

+30
-6
lines changed

1 file changed

+30
-6
lines changed

spec/features/project/updating_a_project_spec.rb

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
let(:headers) { { Authorization: UserProfileMock::TOKEN } }
77
let(:project_type) { Project::Types::PYTHON }
88
let(:user_id) { owner.id }
9-
let!(:project) { create(:project, name: 'Test Project', user_id:, locale: 'en', project_type:) }
9+
let(:locale) { 'en' }
10+
let!(:project) { create(:project, name: 'Test Project', user_id:, locale:, project_type:) }
1011
let(:owner) { create(:owner, school:) }
1112
let(:school) { create(:school) }
1213

@@ -28,31 +29,31 @@
2829
end
2930

3031
it 'responds 200 OK' do
31-
put("/api/projects/#{project.id}", headers:, params:)
32+
put("/api/projects/#{project.identifier}", headers:, params:)
3233
expect(response).to have_http_status(:ok)
3334
end
3435

3536
it 'responds with the project JSON' do
36-
put("/api/projects/#{project.id}", headers:, params:)
37+
put("/api/projects/#{project.identifier}", headers:, params:)
3738
data = JSON.parse(response.body, symbolize_names: true)
3839

3940
expect(data[:name]).to eq('New Name')
4041
end
4142

4243
it 'responds with the components JSON' do
43-
put("/api/projects/#{project.id}", headers:, params:)
44+
put("/api/projects/#{project.identifier}", headers:, params:)
4445
data = JSON.parse(response.body, symbolize_names: true)
4546

4647
expect(data[:components].first[:content]).to eq('print("hello")')
4748
end
4849

4950
it 'responds 422 Unprocessable Entity when params are invalid' do
50-
put("/api/projects/#{project.id}", headers:, params: { project: { components: [{ name: ' ' }] } })
51+
put("/api/projects/#{project.identifier}", headers:, params: { project: { components: [{ name: ' ' }] } })
5152
expect(response).to have_http_status(:unprocessable_entity)
5253
end
5354

5455
it 'responds 401 Unauthorized when no token is given' do
55-
put("/api/projects/#{project.id}", params:)
56+
put("/api/projects/#{project.identifier}", params:)
5657
expect(response).to have_http_status(:unauthorized)
5758
end
5859

@@ -78,4 +79,27 @@
7879
expect(data[:name]).to eq('Test Project')
7980
end
8081
end
82+
83+
context 'when locale is nil, i.e. the other fallback locale in ProjectLoader' do
84+
let(:locale) { nil }
85+
86+
it 'responds 200 OK even though no locale is specified in query string' do
87+
put("/api/projects/#{project.identifier}", headers:, params:)
88+
expect(response).to have_http_status(:ok)
89+
end
90+
end
91+
92+
context "when locale is 'fr', i.e. not a fallback locale in ProjectLoader" do
93+
let(:locale) { 'fr' }
94+
95+
it 'responds 200 OK if locale is specified in query string' do
96+
put("/api/projects/#{project.identifier}?locale=fr", headers:, params:)
97+
expect(response).to have_http_status(:ok)
98+
end
99+
100+
it 'responds 404 Not Found if locale is not specified in query string' do
101+
put("/api/projects/#{project.identifier}", headers:, params:)
102+
expect(response).to have_http_status(:not_found)
103+
end
104+
end
81105
end

0 commit comments

Comments
 (0)