Refactor to use rest API for edit blob
This task is to refactor edit blob to use the public REST API PUT /projects/:id/repository/files/:file_path
instead of the internal controller endpoints.
Implementation notes
Here is some implementation notes and based on the code in !174287 (merged)
- In
handleBlobEdit
inapp/assets/javascripts/repository/pages/blob_edit_header.vue
, update to use the rest endpoint
handleBlobEdit (formData){
...
const originalFormData = Object.fromEntries(formData);
let response;
response = await this.editBlob(originalFormData);
...
},
editBlob(originalFormData) {
const url = buildApiUrl(this.$options.UPDATE_FILE_PATH)
.replace(':id', this.projectId)
.replace(':file_path', this.originalFilePath);
const data = {
branch: originalFormData.branch_name || originalFormData.original_branch,
commit_message: originalFormData.commit_message,
content: originalFormData.file,
file_path: originalFormData.file_path,
id: this.projectId,
last_commit_id: originalFormData.last_commit_sha,
start_branch: originalFormData.original_branch,
};
return axios({
method: 'put',
url,
data,
});
},
handleBlobEdit (formData){
...
const originalFormData = Object.fromEntries(formData);
let response;
response = await this.editBlob(originalFormData);
...
},
editBlob(originalFormData) {
const url = buildApiUrl(this.$options.UPDATE_FILE_PATH)
.replace(':id', this.projectId)
.replace(':file_path', this.originalFilePath);
const data = {
branch: originalFormData.branch_name || originalFormData.original_branch,
commit_message: originalFormData.commit_message,
content: originalFormData.file,
file_path: originalFormData.file_path,
id: this.projectId,
last_commit_id: originalFormData.last_commit_sha,
start_branch: originalFormData.original_branch,
};
return axios({
method: 'put',
url,
data,
});
},
- to make the call above,
project_id
needs to be injected throughedit_blob_app_data
inapp/helpers/blob_helper.rb
- to support the case of updating file name, we need to get the original file path through
app/assets/javascripts/blob_edit/edit_blob.js
and use that in api url
// app/assets/javascripts/blob_edit/edit_blob.js
getOriginalFilePath() {
return this.options.filePath;
}
// app/assets/javascripts/repository/pages/blob_edit_header.vue
openModal() {
...
this.originalFilePath = this.editor.getOriginalFilePath();
...
},
// app/assets/javascripts/blob_edit/edit_blob.js
getOriginalFilePath() {
return this.options.filePath;
}
// app/assets/javascripts/repository/pages/blob_edit_header.vue
openModal() {
...
this.originalFilePath = this.editor.getOriginalFilePath();
...
},
- the api will return an error message in
error.response.data.message
so we need to updatecatch
block in InhandleBlobEdit
inapp/assets/javascripts/repository/pages/blob_edit_header.vue
- if the call is successful, we need to route to the updated file path. this url can be assembled based on
filePath
andbranch
from the api response
handleBlobEdit (formData){
...
visitUrl(this.getUpdatePath(branch, file_path));
...
},
getUpdatePath(branch, filePath) {
const url = new URL(window.location.href);
url.pathname = joinPaths(this.projectPath, '-/blob', branch, filePath);
return url.toString();
},
handleBlobEdit (formData){
...
visitUrl(this.getUpdatePath(branch, file_path));
...
},
getUpdatePath(branch, filePath) {
const url = new URL(window.location.href);
url.pathname = joinPaths(this.projectPath, '-/blob', branch, filePath);
return url.toString();
},
- for the route above, we need to inject
project_path
needs to be injected throughedit_blob_app_data
inapp/helpers/blob_helper.rb
- to handle create merge request, we want to check the
create_merge_request
value from original form data and open the new merge request route-/merge_requests/new
(note, i didn't dig deeper into how to make sure all necessary params are passed in correctly). - this should leave us with unused injections in
edit_blob_app_data
inapp/helpers/blob_helper.rb
, which we can clean up. - we should also be able to clean up logic to show error message in haml file, such as
app/views/projects/blob/edit.html.haml
Test cases
- All rspec should pass
- See MR descriptions !174287 (merged) and !170512 (merged) for more test cases
Edited by 🤖 GitLab Bot 🤖