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

/convert endpoint is not setting default language to en when language is omitted #166

Closed
tagliala opened this issue Oct 5, 2022 · 1 comment · Fixed by #316
Closed
Assignees
Labels

Comments

@tagliala
Copy link
Member

tagliala commented Oct 5, 2022

# language - the language of the file (defaults to 'en')

When params[:language] is omitted, convert_file will be called with nil, and the fallback on English will not work

content = Converter.new(logger:@logger).convert_file( params[:action], body, params[:language] )

This can be fixed in two ways

  1. Change signature of Heathen::Converter.convert
diff --git a/lib/heathen/converter.rb b/lib/heathen/converter.rb
index 11f064f..52e7d85 100644
--- a/lib/heathen/converter.rb
+++ b/lib/heathen/converter.rb
@@ -13,7 +13,8 @@ module Heathen
     # @param content [String] the document body to be converted
     # @param language [String] the document langauge (defaults to 'en')
     # @return [String] the converted document body
-    def convert action, content, language='en'
+    def convert action, content, language=nil
+      language ||= 'en'
       job = Job.new action, content, language
       processor = Heathen::Processor.new job: job, logger: @logger
       begin
  1. OR fix app.rb by adding params.fetch(:language, 'en')
diff --git a/lib/app.rb b/lib/app.rb
index 6d537a4..358183e 100644
--- a/lib/app.rb
+++ b/lib/app.rb
@@ -196,7 +196,7 @@ module Colore
         end
 
         body = params[:file][:tempfile].read
-        content = Converter.new(logger:@logger).convert_file( params[:action], body, params[:language] )
+        content = Converter.new(logger:@logger).convert_file( params[:action], body, params.fetch(:language, 'en') )
         content_type content.mime_type
         content
       rescue StandardError => e
@@ -221,7 +221,7 @@ module Colore
         else
           raise DocumentNotFound.new "Please specify either 'file' or 'url' POST variable"
         end
-        path = LegacyConverter.new.convert_file params[:action], body, params[:language]
+        path = LegacyConverter.new.convert_file params[:action], body, params.fetch(:language, 'en')
         converted_url = @legacy_url_base + path
         content_type 'application/json'
         {
@tagliala tagliala added the bug label Oct 5, 2022
@lleirborras
Copy link
Member

I would go with option 1 as it will make it consistent for the future use of the methoid

@tagliala tagliala self-assigned this Oct 5, 2022
tagliala added a commit that referenced this issue Oct 5, 2022
tagliala added a commit that referenced this issue Oct 5, 2022
tagliala added a commit that referenced this issue Sep 27, 2024
tagliala added a commit that referenced this issue Sep 27, 2024
- Add spec for #166
- Use `and_return`
@tagliala tagliala mentioned this issue Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants