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

fix: repair JSON export #51

Merged
merged 11 commits into from
Nov 24, 2023
Merged

fix: repair JSON export #51

merged 11 commits into from
Nov 24, 2023

Conversation

PascalinDe
Copy link
Member

@PascalinDe PascalinDe commented Nov 9, 2023

This PR fixes the various problems w/ the exported JSON, including

  • cleaning up (some) internalization: Title and Hint
  • removing an internal data structure from the exported JSON

closes dedis#316

@PascalinDe PascalinDe marked this pull request as draft November 9, 2023 17:26
@PascalinDe PascalinDe force-pushed the 316 branch 2 times, most recently from 3c8c021 to 768768f Compare November 16, 2023 09:27
@PascalinDe PascalinDe changed the title refactor: use Title object instead of MainTitle, TitleDe and TitleFr fix: repair JSON export Nov 21, 2023
@PascalinDe PascalinDe marked this pull request as ready for review November 21, 2023 16:41
@PascalinDe PascalinDe requested a review from ineiti November 21, 2023 16:41
Copy link
Member

@ineiti ineiti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs to be rebased on the latest main.

And probably the languages should have been implemented as a map, not as fixed fields. Imagine if we'd have to add Italian, Rumantsch, Chinese, ...

OK, the correct German translations are not necessary, but still nice.

There will be a lot of empty spaces if a language has been chosen which has not been entered. Should we care about that? Isn't there an i18n method which could help, so we can tell it to do fallbacks on different languages, and it will return the 'best'?

contracts/evoting/controller/action.go Show resolved Hide resolved
web/frontend/src/mocks/mockData.ts Outdated Show resolved Hide resolved
Scaffold: [
{
ID: (0xa2ab).toString(),
Title: '{ "en" : "Rate the course", "fr" : "Note la course", "de" : "Rate the course"}',
Title: { En: 'Rate the course', Fr: 'Note la course', De: 'Rate the course' },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Title: { En: 'Rate the course', Fr: 'Note la course', De: 'Rate the course' },
Title: { En: 'Rate the course', Fr: 'Notez le cours', De: 'Benoten Sie den Kurs' },

web/frontend/src/mocks/mockData.ts Outdated Show resolved Hide resolved
web/frontend/src/mocks/mockData.ts Outdated Show resolved Hide resolved
web/frontend/src/pages/form/components/FormForm.tsx Outdated Show resolved Hide resolved
web/frontend/src/pages/form/components/FormForm.tsx Outdated Show resolved Hide resolved
web/frontend/src/types/getObjectType.ts Outdated Show resolved Hide resolved
Copy link
Member

@ineiti ineiti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. please add my changes for de
  2. Uploading the form via JSON does work - Hurray!
  3. ./scripts/local_form.sh should create a correct form. I tried to run it with the following JSON on line 7 (I had to add Using correct URL #61 to make it work, which is normal, but just in case you want to repeat my experiment):
    {"Title":{"En":"Colours","Fr":"","De":""},"Scaffold":[{"ID":"GErn0cNj","Title":{"En":"Colours","Fr":"","De":""},"Order":["DLfAvGgv"],"Ranks":[{"ID":"DLfAvGgv","Title":{"En":"RGB","Fr":"","De":""},"MaxN":3,"MinN":3,"Choices":["{\"en\":\"Red\"}","{\"en\":\"Green\"}","{\"en\":\"Blue\"}"],"Hint":{"En":"","Fr":"","De":""}}],"Selects":[],"Texts":[],"Subjects":[]}]}
    a. To reproduce this, run ./scripts/run_local.sh, then ./scripts/local_proxies.sh, and ./scripts/local_forms.sh with the changed JSON
    b. It adds the form, but when I try to vote, the screen is empty
    c. In the JSON of the 'ranking' vote above, the Choices still looks like an escaped string and not a JSON - is this on purpose?
    d. The title of the form created with local_form.sh is title, and not Colours
    e. When trying to vote on this form, I have the following error in my javascript console:
Uncaught TypeError: configObj.Scaffold is not iterable
    at unmarshalConfigAndCreateAnswers (JSONparser.ts:171:1)
    at useConfiguration.tsx:13:1

@PascalinDe
Copy link
Member Author

  1. please add my changes for de

    1. Uploading the form via JSON does work - Hurray!

    2. ./scripts/local_form.sh should create a correct form. I tried to run it with the following JSON on line 7 (I had to add Using correct URL #61 to make it work, which is normal, but just in case you want to repeat my experiment):
      {"Title":{"En":"Colours","Fr":"","De":""},"Scaffold":[{"ID":"GErn0cNj","Title":{"En":"Colours","Fr":"","De":""},"Order":["DLfAvGgv"],"Ranks":[{"ID":"DLfAvGgv","Title":{"En":"RGB","Fr":"","De":""},"MaxN":3,"MinN":3,"Choices":["{\"en\":\"Red\"}","{\"en\":\"Green\"}","{\"en\":\"Blue\"}"],"Hint":{"En":"","Fr":"","De":""}}],"Selects":[],"Texts":[],"Subjects":[]}]}
      a. To reproduce this, run ./scripts/run_local.sh, then ./scripts/local_proxies.sh, and ./scripts/local_forms.sh with the changed JSON
      b. It adds the form, but when I try to vote, the screen is empty
      c. In the JSON of the 'ranking' vote above, the Choices still looks like an escaped string and not a JSON - is this on purpose?
      d. The title of the form created with local_form.sh is title, and not Colours
      e. When trying to vote on this form, I have the following error in my javascript console:

Uncaught TypeError: configObj.Scaffold is not iterable
    at unmarshalConfigAndCreateAnswers (JSONparser.ts:171:1)
    at useConfiguration.tsx:13:1
  1. the translations? as I put in a comment somewhere, we'll have to clean up that anyway along w/ all the other translations - since they are not the reason that the JSON is broken I'd like to try to limit this PR to that
  2. I created a form w/ the interface, exported it and re-imported it, and it worked fine on my side (uploading, voting and displaying results)
  3. I'll look into that
    a. + b. + d. + e. cf. 2 so it must be a problem w/ the script's output
    c. Choices is still a string since Choices/ChoicesMap is a bit more difficult to refactor, so I'm letting that aside for the moment

@ineiti
Copy link
Member

ineiti commented Nov 23, 2023

  1. the translations? as I put in a comment somewhere, we'll have to clean up that anyway along w/ all the other translations - since they are not the reason that the JSON is broken I'd like to try to limit this PR to that
  2. I created a form w/ the interface, exported it and re-imported it, and it worked fine on my side (uploading, voting and displaying results)
  3. I'll look into that
    a. + b. + d. + e. cf. 2 so it must be a problem w/ the script's output
    c. Choices is still a string since Choices/ChoicesMap is a bit more difficult to refactor, so I'm letting that aside for the moment
  1. just the ones I translated for the mockData.ts
  2. I need to write something so I can have a 3. afterwards...
  3. yes, please have a look why this doesn't work anymore. I do still use it to test the front/backend manually, so it would be nice if it still works

@PascalinDe PascalinDe requested a review from ineiti November 23, 2023 14:30
@PascalinDe
Copy link
Member Author

1. just the ones I translated for the `mockData.ts`

made up my own translations since that's quicker than copying them, hope that's OK

@ineiti
Copy link
Member

ineiti commented Nov 24, 2023

1. just the ones I translated for the `mockData.ts`

made up my own translations since that's quicker than copying them, hope that's OK

Works for me. Github allows you to "Merge comment" if it's a code change. So you can simply commit all comments. Sometimes there is even "Add to merge batch" or something like that, but I don't see it always, gotta figure out when it appears...

@PascalinDe PascalinDe merged commit 7f8c477 into main Nov 24, 2023
11 checks passed
@PascalinDe PascalinDe deleted the 316 branch November 24, 2023 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

JSON schema created by application is not accepted by application
2 participants