TAN-928 Wrong author copy #7876

sebastienhoorens · 2024-05-13T15:47:23Z

The issue is that the author and budget labels are always shown in English and is related to this code in UiSchemaGeneratorService and InputUiSchemaGeneratorService:

def generate_for(fields)
  locales.index_with do |locale|
    I18n.with_locale(locale) do
      generate_for_current_locale fields
    end
  end
end

The previous code would add the author and budget fields to the generated schemas and thereby skip the I18n.with_locale(locale) part. By adding the author and budget to the fields at the beginning, this issue was solved and the code simplified.

I also did some refactoring:

Getting the participation_method inside the schema services felt like a code smell, so the participation_method was passed as a parameter instead.
The ideas_spec file was way too long with many inconsistencies and some duplicate examples. Some cleanup was done by splitting up the file. However, the diff for the ideas_spec file remains very unreadable. Apologies for that, I didn't really change anything significant, just started some cleanup before realizing it's better to split up this file.

Changelog

Fixed

[TAN-928] The labels of the author and budget fields use the translated copy for the current locale instead of the English copy.

…o/citizenlab into TAN-928-wrong-author-copy

notion-workspace · 2024-05-13T15:47:38Z

🐛 ‘Author’ in the input form is not translated

cl-dev-bot · 2024-05-13T16:10:42Z

	Warnings
⚠️	The PR title contains no Jira issue key (case-sensitive)
⚠️	The branch name contains no Jira issue key (case-sensitive)

	Messages
📖	Changelog provided 🎉
📖	Run the e2e tests
📖	Check translation progress

Generated by 🚫 dangerJS against 49e4f6b

sebastienhoorens · 2024-05-13T16:32:20Z

back/app/services/json_forms_service.rb

-        end
+  def each_schema_with_locale(schema_multiloc)
+    schema_multiloc.each do |locale, schema|
+      I18n.with_locale(locale) do


This is the fix.

… schemas at end (to avoid locale switching issues)

sebastienhoorens · 2024-05-15T16:45:27Z

back/app/controllers/web_api/v1/ideas_controller.rb

@@ -13,7 +13,7 @@ class WebApi::V1::IdeasController < ApplicationController
  def json_forms_schema
    input = Idea.find params[:id]
    enabled_fields = IdeaCustomFieldsService.new(input.custom_form).enabled_fields
-    json_attributes = JsonFormsService.new.input_ui_and_json_multiloc_schemas enabled_fields, current_user, input.input_term
+    json_attributes = JsonFormsService.new.input_ui_and_json_multiloc_schemas enabled_fields, current_user, input.participation_method_on_creation, input.input_term


The service would extract the participation method from the participation context of the custom form. This is exactly what participation_method_on_creation does.

from the participation context of the custom form

@sebastienhoorens Does this mean that the form object itself stores some information about the type of participation method? Or is this extracting the participation method based on the current phase? 🤔

@amanda-anderson The custom form is linked to either a project, or a phase (which is still called the participation context, but actually you now always need a phase to participate). This is then passed to participation_method_for in factory.rb. The participation methods are separate classes, that are not persisted in the database.

For custom fields, there are two situations:

Ideation: Then the participation context of the custom form is always the project, and when given a project participation_method_for will always create an ideation method

Native survey: Then the participation context is always a phase and participation_method_for will create a native survey participation method.

Actually I'm not sure what would happen if an admin created an input in a voting phase (I think this is possible?). I'll take a look at this case to make sure this situation also still works.

Yeah my only thought here was about admins adding inputs (to a specific phase) during other phases in the project :)

Just tested it, and it still works the same for voting phases :)

sebastienhoorens · 2024-05-15T16:47:57Z

back/spec/acceptance/ideas_json_forms_schema_spec.rb

+      context 'when admin' do
+        before { admin_header_token }
+
+        example 'Get the json schema and ui schema for an ideation input with author field', document: false do


This is the spec that caches the bug.

amanda-anderson · 2024-05-16T08:10:19Z

back/app/services/json_forms_service.rb

-    end
-    json_schema_multiloc = InputJsonSchemaGeneratorService.new.generate_for visible_fields
-    ui_schema_multiloc = InputUiSchemaGeneratorService.new(input_term, supports_answer_visible_to).generate_for visible_fields
+    fields.reject!(&:hidden?)


I don't completely understand the changes here. Before, were we rejecting fields if they were not enable or hidden? And now we're only rejecting them if they're hidden? 🤔

So before, we filtered out these values and assigned the remaining fields to "visible_fields", but we don't need to do that anymore then?

Yes indeed, actually I think this service should never do any filtering on the fields and should only care about generating the corresponding schemas for whichever fields are passed. Then it's the caller's responsibility to pass the right fields. In the end, I added back fields.reject!(&:hidden?) because this is more of an explicit requirement (for inputs, the hidden attribute is only used to hide their fields from the form/schema), but I hope one day we can remove this line.

An alternative could be to pass the custom form, and let the JsonFormsService determine the fields, but that would be less consistent with the registration (user) fields, and I hope that we can unify the code of the schema's for both one day.

Ok gotchya, thank you! :)

amanda-anderson

Changes make sense to me I think - just had one question about the enabled/hidden change in json_forms_service.rb, but happy to approve in advance.

Should we run the E2E tests for this branch maybe as well just to confirm they're still passing? If you merge in the latest master, the E2E tests should be stable now :)

sebastienhoorens · 2024-05-16T08:22:13Z

Should we run the E2E tests for this branch maybe as well just to confirm they're still passing?

@amanda-anderson Sounds good! I did some manual testing, but it makes sense to run the E2E tests. Thanks for the review!

sebastienhoorens and others added 5 commits May 13, 2024 16:26

imporved specs for author and budget fields

fb2e9fb

Translations updated by CI (extract-intl)

0eaeca0

spec for author label issue + fix

beaa240

Merge branch 'TAN-928-wrong-author-copy' of github.com:CitizenLabDotC…

5c95faa

…o/citizenlab into TAN-928-wrong-author-copy

refactoring: separate methods for adding author and budget to schemas

be2ca0f

move admin post requests to admin context

5a5b51b

moved admin patch to admin context

9805068

sebastienhoorens commented May 13, 2024

View reviewed changes

sebastienhoorens added 20 commits May 14, 2024 15:27

small cleanup

9278ca4

insert author and budget in list of fields at beginning instead of in…

85ef8ff

… schemas at end (to avoid locale switching issues)

remove old code

e05e2e6

manual merge

b69a1e5

pass participation_method as argument to get input schemas

10ceae2

fix offenses

1c999db

fixes after refactoring

010d1a8

fixes after refactoring

6dc62e4

moved idea post specs to separate file

aeebb5a

reorganize ideas post specs by method

7712aa5

minor cleanup

2957814

move admin idea post

f2f2d0c

exclude some admin post idea specs from docs + cleanup

07d89c7

fix offenses

1e1013d

ideas schemas spec

86adb46

ideation context in schema specs

2717a9c

initial ideas update spec

08417f3

moved various admin input update specs

6f86fe1

moved all admin idea update specs

6ad542a

moved all author idea update specs

2a3ec04

sebastienhoorens added 3 commits May 15, 2024 18:14

moved all moderator idea update specs

45ff780

hidden for custom forms means exclude from schema

f03f9dc

split up ideas update spec

deecccf

sebastienhoorens commented May 15, 2024

View reviewed changes

sebastienhoorens added 3 commits May 15, 2024 18:54

fix offenses

11c9ee3

Merge branch 'master' into TAN-928-wrong-author-copy

3d3354f

revert ideas_spec.rb

50bb452

amanda-anderson reviewed May 16, 2024

View reviewed changes

amanda-anderson approved these changes May 16, 2024

View reviewed changes

fix offenses

49e4f6b

sebastienhoorens merged commit 9108d83 into master May 16, 2024
9 of 10 checks passed

sebastienhoorens deleted the TAN-928-wrong-author-copy branch May 16, 2024 10:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TAN-928 Wrong author copy #7876

TAN-928 Wrong author copy #7876

sebastienhoorens commented May 13, 2024 •

edited

notion-workspace bot commented May 13, 2024

cl-dev-bot commented May 13, 2024 •

edited

sebastienhoorens May 13, 2024

sebastienhoorens May 15, 2024

amanda-anderson May 16, 2024

sebastienhoorens May 16, 2024

amanda-anderson May 16, 2024

sebastienhoorens May 16, 2024

sebastienhoorens May 15, 2024

amanda-anderson May 16, 2024

amanda-anderson May 16, 2024

sebastienhoorens May 16, 2024 •

edited

amanda-anderson May 16, 2024

amanda-anderson left a comment

sebastienhoorens commented May 16, 2024

TAN-928 Wrong author copy #7876

TAN-928 Wrong author copy #7876

Conversation

sebastienhoorens commented May 13, 2024 • edited

Changelog

Fixed

notion-workspace bot commented May 13, 2024

cl-dev-bot commented May 13, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebastienhoorens May 16, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amanda-anderson left a comment

Choose a reason for hiding this comment

sebastienhoorens commented May 16, 2024

sebastienhoorens commented May 13, 2024 •

edited

cl-dev-bot commented May 13, 2024 •

edited

sebastienhoorens May 16, 2024 •

edited