From 4435f85c29810f0254c84f6ffc21e452dfd73cd8 Mon Sep 17 00:00:00 2001 From: Daniel Bengl Date: Tue, 5 Nov 2024 06:35:00 +0100 Subject: [PATCH 01/11] Refactor translation files controller Revert "Update json tests" This reverts commit 7ff1ffdc7e718c75a7297cbdda699bd40c6f81e2. Refactor controller Fix specs Fix linter Add api test Update translation_files_controller Fix linter --- .../moirai/translation_files_controller.rb | 93 +++++++---- .../translation_files_controller_api_test.rb | 148 ++++++++++++++++++ .../translation_files_controller_test.rb | 80 +++++----- 3 files changed, 252 insertions(+), 69 deletions(-) create mode 100644 test/controllers/moirai/translation_files_controller_api_test.rb diff --git a/app/controllers/moirai/translation_files_controller.rb b/app/controllers/moirai/translation_files_controller.rb index 3ae9b2c..fcff50a 100644 --- a/app/controllers/moirai/translation_files_controller.rb +++ b/app/controllers/moirai/translation_files_controller.rb @@ -11,12 +11,22 @@ def index path: path } end + + respond_to do |format| + format.html { render :index } + format.json { render json: @files } + end end def show @translation_keys = @file_handler.parse_file(@file_path) @locale = @file_handler.get_first_key(@file_path) @translations = Moirai::Translation.by_file_path(@file_path) + + respond_to do |format| + format.html { render :show } + format.json { render json: {translation_keys: @translation_keys, locale: @locale, translations: @translations} } + end end def create_or_update @@ -28,70 +38,91 @@ def create_or_update end def open_pr - flash.notice = "I created an amazing Pull Request" - changes = Moirai::TranslationDumper.new.call + changes = Moirai::TranslatDumper.new.call Moirai::PullRequestCreator.new.create_pull_request(changes) - redirect_back_or_to(root_path) + + respond_to do |format| + format.html { redirect_back_or_to(root_path, notice: "I created an amazing Pull Request") } + format.json { render json: {message: "Pull Request created"}, status: :ok } + end end private def handle_update(translation) if translation_params[:value].blank? || translation_same_as_current? + fallback_translation = {key: translation.key, value: translation.value} translation.destroy - flash.notice = "Translation #{translation.key} was successfully deleted." - redirect_to_translation_file(translation.file_path) + respond_to do |format| + format.html { redirect_to_translation_file(translation.file_path, notice: "Translation #{translation.key} was successfully deleted.") } + format.json { render json: {message: "Translation deleted", fallback_translation: fallback_translation}, status: :ok } + end return end if translation.update(value: translation_params[:value]) - flash.notice = "Translation #{translation.key} was successfully updated." + respond_to do |format| + format.html { redirect_to_translation_file(translation.file_path, notice: "Translation #{translation.key} was successfully updated.") } + format.json { render json: {message: "Translation updated", translation: translation}, status: :ok } + end else - flash.alert = translation.errors.full_messages.join(", ") + respond_to do |format| + format.html { + flash.now[:alert] = translation.errors.full_messages.join(", ") + redirect_back(fallback_location: root_path) + } + format.json { render json: {errors: translation.errors.full_messages}, status: :unprocessable_entity } + end end - - success_response(translation) end def handle_create if translation_same_as_current? - flash.alert = "Translation #{translation_params[:key]} already exists." - redirect_back_or_to moirai_translation_files_path, status: :unprocessable_entity + respond_to do |format| + format.html { + flash.now[:alert] = "Translation #{translation_params[:key]} already exists." + redirect_back(fallback_location: root_path) + } + format.json { render json: {errors: ["Translation already exists"]}, status: :unprocessable_entity } + end return end translation = Translation.new(translation_params) if translation.save - flash.notice = "Translation #{translation.key} was successfully created." - success_response(translation) + respond_to do |format| + format.html { redirect_to_translation_file(translation.file_path, notice: "Translation #{translation.key} was successfully created.") } + format.json { render json: {message: "Translation created", translation: translation}, status: :ok } + end else - flash.alert = translation.errors.full_messages.join(", ") - redirect_back_or_to moirai_translation_files_path, status: :unprocessable_entity + respond_to do |format| + format.html { + flash.now[:alert] = translation.errors.full_messages.join(", ") + redirect_back(fallback_location: root_path) + } + format.json { render json: {errors: translation.errors.full_messages}, status: :unprocessable_entity } + end end end - def success_response(translation) + def redirect_to_translation_file(file_path, notice: nil) respond_to do |format| - format.json do - flash.discard - render json: {} - end - format.all do - redirect_to_translation_file(translation.file_path) - end + format.html { redirect_to moirai_translation_file_path(Digest::SHA256.hexdigest(file_path)), notice: notice } + format.json { render json: {message: notice} } end end - def redirect_to_translation_file(file_path) - redirect_back_or_to moirai_translation_file_path(Digest::SHA256.hexdigest(file_path)) - end - def set_translation_file @file_path = @file_handler.file_hashes[params[:hashed_file_path]] if @file_path.nil? - flash.alert = "File not found" - redirect_to moirai_translation_files_path, status: :not_found + respond_to do |format| + format.html { + flash[:alert] = "File not found" + redirect_to moirai_translation_files_path + } + format.json { render json: {errors: ["File not found"]}, status: :not_found } + end end end @@ -103,10 +134,6 @@ def load_file_handler @file_handler = Moirai::TranslationFileHandler.new end - # TODO: to resolve the last point of the TODOs we could look at the current translation (without moirai) - # I quickly tried but I need to use the original backend instead of the moirai one - # The problem is that if we set a value that is the same as currently being used via fallback, - # it will create an entry in the database, and afterwards will try to add it in the PR, which we don't want. def translation_same_as_current? file_paths = KeyFinder.new.file_paths_for(translation_params[:key], locale: translation_params[:locale]) diff --git a/test/controllers/moirai/translation_files_controller_api_test.rb b/test/controllers/moirai/translation_files_controller_api_test.rb new file mode 100644 index 0000000..0b81762 --- /dev/null +++ b/test/controllers/moirai/translation_files_controller_api_test.rb @@ -0,0 +1,148 @@ +require "test_helper" + +class TranslationFilesControllerTest < ActionDispatch::IntegrationTest + setup do + @translation = Moirai::Translation.create(key: "locales.german", locale: "de", value: "Neudeutsch") + end + + # Index action tests + test "index displays translation files" do + get index_url, as: :json + + assert_response :success + json_response = JSON.parse(response.body) + assert json_response.any? { |file| file["name"] == "de.yml" } + assert json_response.any? { |file| file["name"] == "en.yml" } + assert json_response.any? { |file| file["name"] == "it.yml" } + end + + # Show action tests + test "show existing translation file" do + get translation_file_url("config/locales/en.yml"), as: :json + assert_response :success + + json_response = JSON.parse(response.body) + assert_equal "en", json_response["locale"] + assert json_response["translation_keys"].is_a?(Hash) + assert json_response["translations"].is_a?(Array) + end + + test "show non-existing translation file" do + get translation_file_url("does_not_exist.yml"), as: :json + assert_response :not_found + end + + # Create action tests + test "create translation with valid params" do + translation_count_before = Moirai::Translation.count + post translation_files_url, params: {translation: {key: "locales.german", locale: "en", value: "New Translation"}}, as: :json + + assert_response :ok + json_response = JSON.parse(response.body) + assert_equal "Translation created", json_response["message"] + assert_equal "locales.german", json_response["translation"]["key"] + assert_equal "New Translation", json_response["translation"]["value"] + assert_equal translation_count_before + 1, Moirai::Translation.count + end + + test "create translation with existing value" do + translation_count_before = Moirai::Translation.count + + post translation_files_url, params: {translation: {key: "locales.german", locale: "en", value: "German"}}, as: :json + + assert_response :unprocessable_entity + json_response = JSON.parse(response.body) + assert_includes json_response["errors"], "Translation already exists" + assert_equal translation_count_before, Moirai::Translation.count + end + + test "create translation with invalid params" do + translation_count_before = Moirai::Translation.count + + post translation_files_url, params: {translation: {key: "", locale: "", value: ""}}, as: :json + + assert_response :unprocessable_entity + json_response = JSON.parse(response.body) + assert_includes json_response["errors"], "Key can't be blank" + assert_includes json_response["errors"], "Locale can't be blank" + assert_includes json_response["errors"], "Value can't be blank" + assert_equal translation_count_before, Moirai::Translation.count + end + + # Update action tests + test "update translation with blank value" do + count_before = Moirai::Translation.count + post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: ""}}, as: :json + + assert_response :ok + json_response = JSON.parse(response.body) + assert_equal "Translation deleted", json_response["message"] + assert_equal count_before - 1, Moirai::Translation.count + end + + test "update translation with non-blank new value" do + post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: "Hochdeutsch"}}, as: :json + + assert_response :ok + json_response = JSON.parse(response.body) + assert_equal "Translation updated", json_response["message"] + assert_equal "locales.german", json_response["translation"]["key"] + assert_equal "Hochdeutsch", json_response["translation"]["value"] + end + + test "update translation with value from file" do + count_before = Moirai::Translation.count + post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: "Deutsch"}}, as: :json + + assert_response :ok + json_response = JSON.parse(response.body) + assert_equal "Translation deleted", json_response["message"] + assert_equal count_before - 1, Moirai::Translation.count + end + + test "creates a pull request with all the file changes" do + Moirai::Translation.create!(key: "locales.italian", + locale: "de", + value: "Italianese") + + Moirai::Translation.create!(key: "locales.italian", + locale: "it", + value: "Italianese") + + post moirai.moirai_open_pr_path, as: :json + + assert_response :ok + json_response = JSON.parse(response.body) + assert_equal "Pull Request created", json_response["message"] + + @pull_request_creator = Moirai::PullRequestCreator.new + + pr = @pull_request_creator.existing_open_pull_request + + assert pr + file = @pull_request_creator.github_client.contents(@pull_request_creator.github_repo_name, + path: "./config/locales/it.yml", + ref: @pull_request_creator.branch_name) + pr_file_content = Base64.decode64(file.content) + proposed_translations = YAML.load(pr_file_content, symbolize_names: true) + assert "Italianese", proposed_translations.dig(:it, :locales, :italian) + + @pull_request_creator.cleanup + refute @pull_request_creator.existing_open_pull_request + end + + private + + def index_url + "/moirai" + end + + def translation_files_url + "/moirai/translation_files" + end + + def translation_file_url(local_path) + absolute_path = Rails.root.join(local_path).to_s + "/moirai/translation_files/#{Digest::SHA256.hexdigest(absolute_path)}" + end +end diff --git a/test/controllers/moirai/translation_files_controller_test.rb b/test/controllers/moirai/translation_files_controller_test.rb index bc6da81..cb73771 100644 --- a/test/controllers/moirai/translation_files_controller_test.rb +++ b/test/controllers/moirai/translation_files_controller_test.rb @@ -1,96 +1,102 @@ require "test_helper" -class TranslationFilesControllerTest < ActionDispatch::IntegrationTest +class TranslationFilesControllerJsonTest < ActionDispatch::IntegrationTest setup do @translation = Moirai::Translation.create(key: "locales.german", locale: "de", value: "Neudeutsch") end # Index action tests test "index displays translation files" do - get index_url + get index_url, as: :json assert_response :success - assert_select "h1", "Translation files" - assert_includes response.body, "de.yml" - assert_includes response.body, "en.yml" - assert_includes response.body, "it.yml" + json_response = JSON.parse(response.body) + assert json_response.any? { |file| file["name"] == "de.yml" } + assert json_response.any? { |file| file["name"] == "en.yml" } + assert json_response.any? { |file| file["name"] == "it.yml" } end # Show action tests test "show existing translation file" do - get translation_file_url("config/locales/en.yml") + get translation_file_url("config/locales/en.yml"), as: :json assert_response :success - assert_select "h1", "Update translations" - assert_select "code", "./config/locales/en.yml" + json_response = JSON.parse(response.body) + assert_equal "en", json_response["locale"] + assert json_response["translation_keys"].is_a?(Hash) + assert json_response["translations"].is_a?(Array) end test "show non-existing translation file" do - get translation_file_url("does_not_exist.yml") + get translation_file_url("does_not_exist.yml"), as: :json assert_response :not_found end # Create action tests test "create translation with valid params" do translation_count_before = Moirai::Translation.count - post translation_files_url, params: {translation: {key: "locales.german", locale: "en", value: "New Translation"}} + post translation_files_url, params: {translation: {key: "locales.german", locale: "en", value: "New Translation"}}, as: :json - assert_response :redirect - assert_redirected_to translation_file_url("config/locales/en.yml") - assert_equal "Translation locales.german was successfully created.", flash[:notice] - assert_equal Moirai::Translation.last.key, "locales.german" - assert_equal Moirai::Translation.last.value, "New Translation" + assert_response :ok + json_response = JSON.parse(response.body) + assert_equal "Translation created", json_response["message"] + assert_equal "locales.german", json_response["translation"]["key"] + assert_equal "New Translation", json_response["translation"]["value"] assert_equal translation_count_before + 1, Moirai::Translation.count end test "create translation with existing value" do translation_count_before = Moirai::Translation.count - post translation_files_url, params: {translation: {key: "locales.german", locale: "en", value: "German"}} + post translation_files_url, params: {translation: {key: "locales.german", locale: "en", value: "German"}}, as: :json assert_response :unprocessable_entity - assert_equal "Translation locales.german already exists.", flash[:alert] + json_response = JSON.parse(response.body) + assert_includes json_response["errors"], "Translation already exists" assert_equal translation_count_before, Moirai::Translation.count end test "create translation with invalid params" do translation_count_before = Moirai::Translation.count - post translation_files_url, params: {translation: {key: "", locale: "", value: ""}} + post translation_files_url, params: {translation: {key: "", locale: "", value: ""}}, as: :json assert_response :unprocessable_entity - assert_equal "Key can't be blank, Locale can't be blank, Value can't be blank", flash[:alert] + json_response = JSON.parse(response.body) + assert_includes json_response["errors"], "Key can't be blank" + assert_includes json_response["errors"], "Locale can't be blank" + assert_includes json_response["errors"], "Value can't be blank" assert_equal translation_count_before, Moirai::Translation.count end # Update action tests test "update translation with blank value" do count_before = Moirai::Translation.count - post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: ""}} + post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: ""}}, as: :json - assert_response :redirect - assert_redirected_to translation_file_url("config/locales/de.yml") - assert_equal "Translation locales.german was successfully deleted.", flash[:notice] + assert_response :ok + json_response = JSON.parse(response.body) + assert_equal "Translation deleted", json_response["message"] assert_equal count_before - 1, Moirai::Translation.count end test "update translation with non-blank new value" do - post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: "Hochdeutsch"}} + post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: "Hochdeutsch"}}, as: :json - assert_response :redirect - assert_redirected_to translation_file_url("config/locales/de.yml") - assert_equal "Translation locales.german was successfully updated.", flash[:notice] - assert_equal Moirai::Translation.last.key, "locales.german" - assert_equal Moirai::Translation.last.value, "Hochdeutsch" + assert_response :ok + json_response = JSON.parse(response.body) + assert_equal "Translation updated", json_response["message"] + assert_equal "locales.german", json_response["translation"]["key"] + assert_equal "Hochdeutsch", json_response["translation"]["value"] end test "update translation with value from file" do count_before = Moirai::Translation.count - post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: "Deutsch"}} + post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: "Deutsch"}}, as: :json - assert_response :redirect - assert_redirected_to translation_file_url("config/locales/de.yml") - assert_equal "Translation locales.german was successfully deleted.", flash[:notice] + assert_response :ok + json_response = JSON.parse(response.body) + assert_equal "Translation deleted", json_response["message"] assert_equal count_before - 1, Moirai::Translation.count end @@ -103,9 +109,11 @@ class TranslationFilesControllerTest < ActionDispatch::IntegrationTest locale: "it", value: "Italianese") - post moirai.moirai_open_pr_path + post moirai.moirai_open_pr_path, as: :json - assert_response :found + assert_response :ok + json_response = JSON.parse(response.body) + assert_equal "Pull Request created", json_response["message"] @pull_request_creator = Moirai::PullRequestCreator.new From 38a31b566d7009ff41b1b9fe6573bd882bdc4c9a Mon Sep 17 00:00:00 2001 From: Daniel Bengl Date: Wed, 13 Nov 2024 16:55:10 +0100 Subject: [PATCH 02/11] Update stimulus controller --- app/assets/javascripts/moirai_translation_controller.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/app/assets/javascripts/moirai_translation_controller.js b/app/assets/javascripts/moirai_translation_controller.js index 50f0fe6..f7cfca9 100644 --- a/app/assets/javascripts/moirai_translation_controller.js +++ b/app/assets/javascripts/moirai_translation_controller.js @@ -27,6 +27,15 @@ export default class MoiraiTranslationController extends Controller { value: event.target.innerText } }) + }) + .then(response => response.json()) + .then(data => { + if (data?.fallback_translation?.value) { + event.target.innerText = data.fallback_translation.value + } + }) + .catch(error => { + console.error('Error:', error); }); } } From b3b3390a8e07048a6fb0b0591d45840a0844c079 Mon Sep 17 00:00:00 2001 From: Daniel Bengl Date: Wed, 13 Nov 2024 16:57:12 +0100 Subject: [PATCH 03/11] Revert "Refactor translation files controller" This reverts commit a8b6bd96d55af39e1c1dec445539e5532be5fdea. --- .../moirai/translation_files_controller.rb | 92 ++++------- .../translation_files_controller_api_test.rb | 148 ------------------ .../translation_files_controller_test.rb | 80 +++++----- 3 files changed, 68 insertions(+), 252 deletions(-) delete mode 100644 test/controllers/moirai/translation_files_controller_api_test.rb diff --git a/app/controllers/moirai/translation_files_controller.rb b/app/controllers/moirai/translation_files_controller.rb index fcff50a..9c794bc 100644 --- a/app/controllers/moirai/translation_files_controller.rb +++ b/app/controllers/moirai/translation_files_controller.rb @@ -11,22 +11,12 @@ def index path: path } end - - respond_to do |format| - format.html { render :index } - format.json { render json: @files } - end end def show @translation_keys = @file_handler.parse_file(@file_path) @locale = @file_handler.get_first_key(@file_path) @translations = Moirai::Translation.by_file_path(@file_path) - - respond_to do |format| - format.html { render :show } - format.json { render json: {translation_keys: @translation_keys, locale: @locale, translations: @translations} } - end end def create_or_update @@ -38,91 +28,69 @@ def create_or_update end def open_pr - changes = Moirai::TranslatDumper.new.call + flash.notice = "I created an amazing Pull Request" + changes = Moirai::TranslationDumper.new.call Moirai::PullRequestCreator.new.create_pull_request(changes) - - respond_to do |format| - format.html { redirect_back_or_to(root_path, notice: "I created an amazing Pull Request") } - format.json { render json: {message: "Pull Request created"}, status: :ok } - end + redirect_back_or_to(root_path) end private def handle_update(translation) if translation_params[:value].blank? || translation_same_as_current? - fallback_translation = {key: translation.key, value: translation.value} translation.destroy - respond_to do |format| - format.html { redirect_to_translation_file(translation.file_path, notice: "Translation #{translation.key} was successfully deleted.") } - format.json { render json: {message: "Translation deleted", fallback_translation: fallback_translation}, status: :ok } - end + flash.notice = "Translation #{translation.key} was successfully deleted." + redirect_to_translation_file(translation.file_path) return end if translation.update(value: translation_params[:value]) - respond_to do |format| - format.html { redirect_to_translation_file(translation.file_path, notice: "Translation #{translation.key} was successfully updated.") } - format.json { render json: {message: "Translation updated", translation: translation}, status: :ok } - end + flash.notice = "Translation #{translation.key} was successfully updated." else - respond_to do |format| - format.html { - flash.now[:alert] = translation.errors.full_messages.join(", ") - redirect_back(fallback_location: root_path) - } - format.json { render json: {errors: translation.errors.full_messages}, status: :unprocessable_entity } - end + flash.alert = translation.errors.full_messages.join(", ") end + + success_response(translation) end def handle_create if translation_same_as_current? - respond_to do |format| - format.html { - flash.now[:alert] = "Translation #{translation_params[:key]} already exists." - redirect_back(fallback_location: root_path) - } - format.json { render json: {errors: ["Translation already exists"]}, status: :unprocessable_entity } - end + flash.alert = "Translation #{translation_params[:key]} already exists." + redirect_back_or_to moirai_translation_files_path, status: :unprocessable_entity return end translation = Translation.new(translation_params) if translation.save - respond_to do |format| - format.html { redirect_to_translation_file(translation.file_path, notice: "Translation #{translation.key} was successfully created.") } - format.json { render json: {message: "Translation created", translation: translation}, status: :ok } - end + flash.notice = "Translation #{translation.key} was successfully created." + success_response(translation) else - respond_to do |format| - format.html { - flash.now[:alert] = translation.errors.full_messages.join(", ") - redirect_back(fallback_location: root_path) - } - format.json { render json: {errors: translation.errors.full_messages}, status: :unprocessable_entity } - end + flash.alert = translation.errors.full_messages.join(", ") + redirect_back_or_to moirai_translation_files_path, status: :unprocessable_entity end end - def redirect_to_translation_file(file_path, notice: nil) + def success_response(translation) respond_to do |format| - format.html { redirect_to moirai_translation_file_path(Digest::SHA256.hexdigest(file_path)), notice: notice } - format.json { render json: {message: notice} } + format.json do + render json: {} + end + format.all do + redirect_to_translation_file(translation.file_path) + end end end + def redirect_to_translation_file(file_path) + redirect_back_or_to moirai_translation_file_path(Digest::SHA256.hexdigest(file_path)) + end + def set_translation_file @file_path = @file_handler.file_hashes[params[:hashed_file_path]] if @file_path.nil? - respond_to do |format| - format.html { - flash[:alert] = "File not found" - redirect_to moirai_translation_files_path - } - format.json { render json: {errors: ["File not found"]}, status: :not_found } - end + flash.alert = "File not found" + redirect_to moirai_translation_files_path, status: :not_found end end @@ -134,6 +102,10 @@ def load_file_handler @file_handler = Moirai::TranslationFileHandler.new end + # TODO: to resolve the last point of the TODOs we could look at the current translation (without moirai) + # I quickly tried but I need to use the original backend instead of the moirai one + # The problem is that if we set a value that is the same as currently being used via fallback, + # it will create an entry in the database, and afterwards will try to add it in the PR, which we don't want. def translation_same_as_current? file_paths = KeyFinder.new.file_paths_for(translation_params[:key], locale: translation_params[:locale]) diff --git a/test/controllers/moirai/translation_files_controller_api_test.rb b/test/controllers/moirai/translation_files_controller_api_test.rb deleted file mode 100644 index 0b81762..0000000 --- a/test/controllers/moirai/translation_files_controller_api_test.rb +++ /dev/null @@ -1,148 +0,0 @@ -require "test_helper" - -class TranslationFilesControllerTest < ActionDispatch::IntegrationTest - setup do - @translation = Moirai::Translation.create(key: "locales.german", locale: "de", value: "Neudeutsch") - end - - # Index action tests - test "index displays translation files" do - get index_url, as: :json - - assert_response :success - json_response = JSON.parse(response.body) - assert json_response.any? { |file| file["name"] == "de.yml" } - assert json_response.any? { |file| file["name"] == "en.yml" } - assert json_response.any? { |file| file["name"] == "it.yml" } - end - - # Show action tests - test "show existing translation file" do - get translation_file_url("config/locales/en.yml"), as: :json - assert_response :success - - json_response = JSON.parse(response.body) - assert_equal "en", json_response["locale"] - assert json_response["translation_keys"].is_a?(Hash) - assert json_response["translations"].is_a?(Array) - end - - test "show non-existing translation file" do - get translation_file_url("does_not_exist.yml"), as: :json - assert_response :not_found - end - - # Create action tests - test "create translation with valid params" do - translation_count_before = Moirai::Translation.count - post translation_files_url, params: {translation: {key: "locales.german", locale: "en", value: "New Translation"}}, as: :json - - assert_response :ok - json_response = JSON.parse(response.body) - assert_equal "Translation created", json_response["message"] - assert_equal "locales.german", json_response["translation"]["key"] - assert_equal "New Translation", json_response["translation"]["value"] - assert_equal translation_count_before + 1, Moirai::Translation.count - end - - test "create translation with existing value" do - translation_count_before = Moirai::Translation.count - - post translation_files_url, params: {translation: {key: "locales.german", locale: "en", value: "German"}}, as: :json - - assert_response :unprocessable_entity - json_response = JSON.parse(response.body) - assert_includes json_response["errors"], "Translation already exists" - assert_equal translation_count_before, Moirai::Translation.count - end - - test "create translation with invalid params" do - translation_count_before = Moirai::Translation.count - - post translation_files_url, params: {translation: {key: "", locale: "", value: ""}}, as: :json - - assert_response :unprocessable_entity - json_response = JSON.parse(response.body) - assert_includes json_response["errors"], "Key can't be blank" - assert_includes json_response["errors"], "Locale can't be blank" - assert_includes json_response["errors"], "Value can't be blank" - assert_equal translation_count_before, Moirai::Translation.count - end - - # Update action tests - test "update translation with blank value" do - count_before = Moirai::Translation.count - post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: ""}}, as: :json - - assert_response :ok - json_response = JSON.parse(response.body) - assert_equal "Translation deleted", json_response["message"] - assert_equal count_before - 1, Moirai::Translation.count - end - - test "update translation with non-blank new value" do - post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: "Hochdeutsch"}}, as: :json - - assert_response :ok - json_response = JSON.parse(response.body) - assert_equal "Translation updated", json_response["message"] - assert_equal "locales.german", json_response["translation"]["key"] - assert_equal "Hochdeutsch", json_response["translation"]["value"] - end - - test "update translation with value from file" do - count_before = Moirai::Translation.count - post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: "Deutsch"}}, as: :json - - assert_response :ok - json_response = JSON.parse(response.body) - assert_equal "Translation deleted", json_response["message"] - assert_equal count_before - 1, Moirai::Translation.count - end - - test "creates a pull request with all the file changes" do - Moirai::Translation.create!(key: "locales.italian", - locale: "de", - value: "Italianese") - - Moirai::Translation.create!(key: "locales.italian", - locale: "it", - value: "Italianese") - - post moirai.moirai_open_pr_path, as: :json - - assert_response :ok - json_response = JSON.parse(response.body) - assert_equal "Pull Request created", json_response["message"] - - @pull_request_creator = Moirai::PullRequestCreator.new - - pr = @pull_request_creator.existing_open_pull_request - - assert pr - file = @pull_request_creator.github_client.contents(@pull_request_creator.github_repo_name, - path: "./config/locales/it.yml", - ref: @pull_request_creator.branch_name) - pr_file_content = Base64.decode64(file.content) - proposed_translations = YAML.load(pr_file_content, symbolize_names: true) - assert "Italianese", proposed_translations.dig(:it, :locales, :italian) - - @pull_request_creator.cleanup - refute @pull_request_creator.existing_open_pull_request - end - - private - - def index_url - "/moirai" - end - - def translation_files_url - "/moirai/translation_files" - end - - def translation_file_url(local_path) - absolute_path = Rails.root.join(local_path).to_s - "/moirai/translation_files/#{Digest::SHA256.hexdigest(absolute_path)}" - end -end diff --git a/test/controllers/moirai/translation_files_controller_test.rb b/test/controllers/moirai/translation_files_controller_test.rb index cb73771..bc6da81 100644 --- a/test/controllers/moirai/translation_files_controller_test.rb +++ b/test/controllers/moirai/translation_files_controller_test.rb @@ -1,102 +1,96 @@ require "test_helper" -class TranslationFilesControllerJsonTest < ActionDispatch::IntegrationTest +class TranslationFilesControllerTest < ActionDispatch::IntegrationTest setup do @translation = Moirai::Translation.create(key: "locales.german", locale: "de", value: "Neudeutsch") end # Index action tests test "index displays translation files" do - get index_url, as: :json + get index_url assert_response :success - json_response = JSON.parse(response.body) - assert json_response.any? { |file| file["name"] == "de.yml" } - assert json_response.any? { |file| file["name"] == "en.yml" } - assert json_response.any? { |file| file["name"] == "it.yml" } + assert_select "h1", "Translation files" + assert_includes response.body, "de.yml" + assert_includes response.body, "en.yml" + assert_includes response.body, "it.yml" end # Show action tests test "show existing translation file" do - get translation_file_url("config/locales/en.yml"), as: :json + get translation_file_url("config/locales/en.yml") assert_response :success - json_response = JSON.parse(response.body) - assert_equal "en", json_response["locale"] - assert json_response["translation_keys"].is_a?(Hash) - assert json_response["translations"].is_a?(Array) + assert_select "h1", "Update translations" + assert_select "code", "./config/locales/en.yml" end test "show non-existing translation file" do - get translation_file_url("does_not_exist.yml"), as: :json + get translation_file_url("does_not_exist.yml") assert_response :not_found end # Create action tests test "create translation with valid params" do translation_count_before = Moirai::Translation.count - post translation_files_url, params: {translation: {key: "locales.german", locale: "en", value: "New Translation"}}, as: :json + post translation_files_url, params: {translation: {key: "locales.german", locale: "en", value: "New Translation"}} - assert_response :ok - json_response = JSON.parse(response.body) - assert_equal "Translation created", json_response["message"] - assert_equal "locales.german", json_response["translation"]["key"] - assert_equal "New Translation", json_response["translation"]["value"] + assert_response :redirect + assert_redirected_to translation_file_url("config/locales/en.yml") + assert_equal "Translation locales.german was successfully created.", flash[:notice] + assert_equal Moirai::Translation.last.key, "locales.german" + assert_equal Moirai::Translation.last.value, "New Translation" assert_equal translation_count_before + 1, Moirai::Translation.count end test "create translation with existing value" do translation_count_before = Moirai::Translation.count - post translation_files_url, params: {translation: {key: "locales.german", locale: "en", value: "German"}}, as: :json + post translation_files_url, params: {translation: {key: "locales.german", locale: "en", value: "German"}} assert_response :unprocessable_entity - json_response = JSON.parse(response.body) - assert_includes json_response["errors"], "Translation already exists" + assert_equal "Translation locales.german already exists.", flash[:alert] assert_equal translation_count_before, Moirai::Translation.count end test "create translation with invalid params" do translation_count_before = Moirai::Translation.count - post translation_files_url, params: {translation: {key: "", locale: "", value: ""}}, as: :json + post translation_files_url, params: {translation: {key: "", locale: "", value: ""}} assert_response :unprocessable_entity - json_response = JSON.parse(response.body) - assert_includes json_response["errors"], "Key can't be blank" - assert_includes json_response["errors"], "Locale can't be blank" - assert_includes json_response["errors"], "Value can't be blank" + assert_equal "Key can't be blank, Locale can't be blank, Value can't be blank", flash[:alert] assert_equal translation_count_before, Moirai::Translation.count end # Update action tests test "update translation with blank value" do count_before = Moirai::Translation.count - post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: ""}}, as: :json + post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: ""}} - assert_response :ok - json_response = JSON.parse(response.body) - assert_equal "Translation deleted", json_response["message"] + assert_response :redirect + assert_redirected_to translation_file_url("config/locales/de.yml") + assert_equal "Translation locales.german was successfully deleted.", flash[:notice] assert_equal count_before - 1, Moirai::Translation.count end test "update translation with non-blank new value" do - post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: "Hochdeutsch"}}, as: :json + post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: "Hochdeutsch"}} - assert_response :ok - json_response = JSON.parse(response.body) - assert_equal "Translation updated", json_response["message"] - assert_equal "locales.german", json_response["translation"]["key"] - assert_equal "Hochdeutsch", json_response["translation"]["value"] + assert_response :redirect + assert_redirected_to translation_file_url("config/locales/de.yml") + assert_equal "Translation locales.german was successfully updated.", flash[:notice] + assert_equal Moirai::Translation.last.key, "locales.german" + assert_equal Moirai::Translation.last.value, "Hochdeutsch" end test "update translation with value from file" do count_before = Moirai::Translation.count - post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: "Deutsch"}}, as: :json + post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: "Deutsch"}} - assert_response :ok - json_response = JSON.parse(response.body) - assert_equal "Translation deleted", json_response["message"] + assert_response :redirect + assert_redirected_to translation_file_url("config/locales/de.yml") + assert_equal "Translation locales.german was successfully deleted.", flash[:notice] assert_equal count_before - 1, Moirai::Translation.count end @@ -109,11 +103,9 @@ class TranslationFilesControllerJsonTest < ActionDispatch::IntegrationTest locale: "it", value: "Italianese") - post moirai.moirai_open_pr_path, as: :json + post moirai.moirai_open_pr_path - assert_response :ok - json_response = JSON.parse(response.body) - assert_equal "Pull Request created", json_response["message"] + assert_response :found @pull_request_creator = Moirai::PullRequestCreator.new From 41111ed674a1c568bc0eede77176a98908ee1b51 Mon Sep 17 00:00:00 2001 From: Daniel Bengl Date: Wed, 13 Nov 2024 17:00:26 +0100 Subject: [PATCH 04/11] Merge --- app/controllers/moirai/translation_files_controller.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/moirai/translation_files_controller.rb b/app/controllers/moirai/translation_files_controller.rb index 9c794bc..3ae9b2c 100644 --- a/app/controllers/moirai/translation_files_controller.rb +++ b/app/controllers/moirai/translation_files_controller.rb @@ -74,6 +74,7 @@ def handle_create def success_response(translation) respond_to do |format| format.json do + flash.discard render json: {} end format.all do From 45ef6c473a9f8a648db6f45031b2d38ecc99f369 Mon Sep 17 00:00:00 2001 From: Daniel Bengl Date: Wed, 13 Nov 2024 17:14:10 +0100 Subject: [PATCH 05/11] Implement feature --- .../moirai_translation_controller.js | 4 ++-- .../moirai/translation_files_controller.rb | 21 ++++++++++++++++++- .../translation_files_controller_test.rb | 6 ++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/moirai_translation_controller.js b/app/assets/javascripts/moirai_translation_controller.js index f7cfca9..b0e8ca3 100644 --- a/app/assets/javascripts/moirai_translation_controller.js +++ b/app/assets/javascripts/moirai_translation_controller.js @@ -30,8 +30,8 @@ export default class MoiraiTranslationController extends Controller { }) .then(response => response.json()) .then(data => { - if (data?.fallback_translation?.value) { - event.target.innerText = data.fallback_translation.value + if (data?.fallback_translation) { + event.target.innerText = data.fallback_translation } }) .catch(error => { diff --git a/app/controllers/moirai/translation_files_controller.rb b/app/controllers/moirai/translation_files_controller.rb index 3ae9b2c..514b4c5 100644 --- a/app/controllers/moirai/translation_files_controller.rb +++ b/app/controllers/moirai/translation_files_controller.rb @@ -40,7 +40,17 @@ def handle_update(translation) if translation_params[:value].blank? || translation_same_as_current? translation.destroy flash.notice = "Translation #{translation.key} was successfully deleted." - redirect_to_translation_file(translation.file_path) + respond_to do |format| + format.json do + flash.discard + render json: { + fallback_translation: get_fallback_translation + } + end + format.html do + redirect_to_translation_file(translation.file_path) + end + end return end @@ -115,5 +125,14 @@ def translation_same_as_current? translation_params[:value] == @file_handler.parse_file(file_paths.first)[translation_params[:key]] end + + def get_fallback_translation + file_paths = KeyFinder.new.file_paths_for(translation_params[:key], locale: translation_params[:locale]) + + return "" if file_paths.empty? + return "" unless file_paths.all? { |file_path| File.exist?(file_path) } + + @file_handler.parse_file(file_paths.first)[translation_params[:key]] + end end end diff --git a/test/controllers/moirai/translation_files_controller_test.rb b/test/controllers/moirai/translation_files_controller_test.rb index bc6da81..afb3533 100644 --- a/test/controllers/moirai/translation_files_controller_test.rb +++ b/test/controllers/moirai/translation_files_controller_test.rb @@ -74,6 +74,12 @@ class TranslationFilesControllerTest < ActionDispatch::IntegrationTest assert_equal count_before - 1, Moirai::Translation.count end + test "update translation with blank value json" do + post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: ""}}, as: :json + assert_response :ok + assert_equal "Deutsch", JSON.parse(response.body)["translation"] + end + test "update translation with non-blank new value" do post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: "Hochdeutsch"}} From 25111bf65d56a04ef7af958dec81868fae8f2dc7 Mon Sep 17 00:00:00 2001 From: Daniel Bengl Date: Wed, 13 Nov 2024 17:29:58 +0100 Subject: [PATCH 06/11] Fix bug --- .../moirai/translation_files_controller.rb | 15 +++++++++++++++ .../moirai/translation_files_controller_test.rb | 6 ++++++ 2 files changed, 21 insertions(+) diff --git a/app/controllers/moirai/translation_files_controller.rb b/app/controllers/moirai/translation_files_controller.rb index 514b4c5..9f891a8 100644 --- a/app/controllers/moirai/translation_files_controller.rb +++ b/app/controllers/moirai/translation_files_controller.rb @@ -70,6 +70,21 @@ def handle_create return end + if translation_params[:value].blank? + respond_to do |format| + format.json do + render json: { + fallback_translation: get_fallback_translation + } + end + format.html do + flash.alert = "Value can't be blank" + redirect_back_or_to moirai_translation_file_path(Digest::SHA256.hexdigest(translation_params[:key])) + end + end + return + end + translation = Translation.new(translation_params) if translation.save diff --git a/test/controllers/moirai/translation_files_controller_test.rb b/test/controllers/moirai/translation_files_controller_test.rb index afb3533..77e92a5 100644 --- a/test/controllers/moirai/translation_files_controller_test.rb +++ b/test/controllers/moirai/translation_files_controller_test.rb @@ -63,6 +63,12 @@ class TranslationFilesControllerTest < ActionDispatch::IntegrationTest assert_equal translation_count_before, Moirai::Translation.count end + test "create translation empty json" do + post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: ""}}, as: :json + assert_response :ok + assert_equal "Deutsch", JSON.parse(response.body)["fallback_translation"] + end + # Update action tests test "update translation with blank value" do count_before = Moirai::Translation.count From aff9ada728193f567691e1dbff18d219085f328d Mon Sep 17 00:00:00 2001 From: Daniel Bengl Date: Wed, 13 Nov 2024 18:02:29 +0100 Subject: [PATCH 07/11] Refactor --- app/controllers/moirai/translation_files_controller.rb | 10 ++-------- .../moirai/translation_files_controller_test.rb | 2 +- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/app/controllers/moirai/translation_files_controller.rb b/app/controllers/moirai/translation_files_controller.rb index 9f891a8..a0bcb7a 100644 --- a/app/controllers/moirai/translation_files_controller.rb +++ b/app/controllers/moirai/translation_files_controller.rb @@ -70,16 +70,10 @@ def handle_create return end - if translation_params[:value].blank? + if translation_params[:value].blank? && request.format.json? respond_to do |format| format.json do - render json: { - fallback_translation: get_fallback_translation - } - end - format.html do - flash.alert = "Value can't be blank" - redirect_back_or_to moirai_translation_file_path(Digest::SHA256.hexdigest(translation_params[:key])) + render json: {fallback_translation: get_fallback_translation} end end return diff --git a/test/controllers/moirai/translation_files_controller_test.rb b/test/controllers/moirai/translation_files_controller_test.rb index 77e92a5..7df28b2 100644 --- a/test/controllers/moirai/translation_files_controller_test.rb +++ b/test/controllers/moirai/translation_files_controller_test.rb @@ -83,7 +83,7 @@ class TranslationFilesControllerTest < ActionDispatch::IntegrationTest test "update translation with blank value json" do post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: ""}}, as: :json assert_response :ok - assert_equal "Deutsch", JSON.parse(response.body)["translation"] + assert_equal "Deutsch", JSON.parse(response.body)["fallback_translation"] end test "update translation with non-blank new value" do From 65da948fb120afb6812b2d69cd2697f6888f03d7 Mon Sep 17 00:00:00 2001 From: Daniel Bengl <53896675+CuddlyBunion341@users.noreply.github.com> Date: Wed, 13 Nov 2024 18:07:54 +0100 Subject: [PATCH 08/11] Update test/controllers/moirai/translation_files_controller_test.rb --- test/controllers/moirai/translation_files_controller_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/controllers/moirai/translation_files_controller_test.rb b/test/controllers/moirai/translation_files_controller_test.rb index 7df28b2..a6e3eb0 100644 --- a/test/controllers/moirai/translation_files_controller_test.rb +++ b/test/controllers/moirai/translation_files_controller_test.rb @@ -63,7 +63,7 @@ class TranslationFilesControllerTest < ActionDispatch::IntegrationTest assert_equal translation_count_before, Moirai::Translation.count end - test "create translation empty json" do + test "create translation with blank value JSON" do post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: ""}}, as: :json assert_response :ok assert_equal "Deutsch", JSON.parse(response.body)["fallback_translation"] From 7d5d733a0ba728f2bb1a168e0f653ec47b6a48e2 Mon Sep 17 00:00:00 2001 From: Daniel Bengl <53896675+CuddlyBunion341@users.noreply.github.com> Date: Wed, 13 Nov 2024 18:08:07 +0100 Subject: [PATCH 09/11] Update test/controllers/moirai/translation_files_controller_test.rb --- test/controllers/moirai/translation_files_controller_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/controllers/moirai/translation_files_controller_test.rb b/test/controllers/moirai/translation_files_controller_test.rb index a6e3eb0..8584c67 100644 --- a/test/controllers/moirai/translation_files_controller_test.rb +++ b/test/controllers/moirai/translation_files_controller_test.rb @@ -80,7 +80,7 @@ class TranslationFilesControllerTest < ActionDispatch::IntegrationTest assert_equal count_before - 1, Moirai::Translation.count end - test "update translation with blank value json" do + test "update translation with blank value JSON" do post translation_files_url, params: {translation: {key: "locales.german", locale: "de", value: ""}}, as: :json assert_response :ok assert_equal "Deutsch", JSON.parse(response.body)["fallback_translation"] From ea2b2181c718b9ad6ef2fba61acbc7cc42ba16e4 Mon Sep 17 00:00:00 2001 From: Daniel Bengl <53896675+CuddlyBunion341@users.noreply.github.com> Date: Thu, 14 Nov 2024 11:08:19 +0100 Subject: [PATCH 10/11] Update app/controllers/moirai/translation_files_controller.rb --- app/controllers/moirai/translation_files_controller.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/app/controllers/moirai/translation_files_controller.rb b/app/controllers/moirai/translation_files_controller.rb index a0bcb7a..cc132e4 100644 --- a/app/controllers/moirai/translation_files_controller.rb +++ b/app/controllers/moirai/translation_files_controller.rb @@ -71,12 +71,7 @@ def handle_create end if translation_params[:value].blank? && request.format.json? - respond_to do |format| - format.json do - render json: {fallback_translation: get_fallback_translation} - end - end - return + return render json: {fallback_translation: get_fallback_translation} end translation = Translation.new(translation_params) From 452dbf3668a93bc47d0f69b007d858667cf7c9d9 Mon Sep 17 00:00:00 2001 From: Alessandro Rodi Date: Thu, 21 Nov 2024 22:19:44 +0100 Subject: [PATCH 11/11] Do not discard flash message --- app/controllers/moirai/translation_files_controller.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controllers/moirai/translation_files_controller.rb b/app/controllers/moirai/translation_files_controller.rb index cc132e4..a6f35dd 100644 --- a/app/controllers/moirai/translation_files_controller.rb +++ b/app/controllers/moirai/translation_files_controller.rb @@ -39,15 +39,14 @@ def open_pr def handle_update(translation) if translation_params[:value].blank? || translation_same_as_current? translation.destroy - flash.notice = "Translation #{translation.key} was successfully deleted." respond_to do |format| format.json do - flash.discard render json: { fallback_translation: get_fallback_translation } end format.html do + flash.notice = "Translation #{translation.key} was successfully deleted." redirect_to_translation_file(translation.file_path) end end