Conversation
keabard
force-pushed
the
feat/admin/create-event-with-json
branch
from
5692d2b to
a829940
Compare
|
Nice work! Thank you for the PR; I will take a look this week! |
HugoGresse
added
enhancement
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
|
||
| getSpeakers() { | ||
| return this.getJsonData().then((data) => data.speakers) | ||
| } |
There was a problem hiding this comment.
Bug: Incorrect Promise Handling in Data Retrieval Methods
The getTalks(), getTalk(), and getSpeakers() methods call .then() on the result of getJsonData(). However, getJsonData() returns data synchronously, not a Promise, which will cause runtime errors.
| type="text" | ||
| name={t('settingsSetup.json.fieldJson')} | ||
| fieldName="json" | ||
| type="textarea" |
There was a problem hiding this comment.
Bug: Form Field Renaming Causes Validation and Submission Issues
The form field's fieldName was updated from jsonUrl to json, but the validationSchema and onSubmit handler still reference jsonUrl. This causes validation to fail and undefined values to be submitted.
| component="p" | ||
| variant="subtitle1"> | ||
| detail | ||
| </TranslatedTypography> |
There was a problem hiding this comment.
Bug: JSON Project Type Uses Incorrect Translation Keys
The new JSON project type radio button incorrectly uses translation keys newEvent.step2.projectTypeJsonurl and newEvent.step2.projectTypeJsonurlDetail. It should use newEvent.step2.projectTypeJson and newEvent.step2.projectTypeJsonDetail, which are available in the translation files, to display the correct text.
| stepTitle={stepTitle} | ||
| title={t('newEvent.step3.jsonurl')} | ||
| onCancel={onCancel}> | ||
| <SetupJSON |
There was a problem hiding this comment.
Bug: Incorrect Component Used for JSON URLs
The Step3JSONUrl component uses SetupJSON, which is designed for direct JSON input. As Step3JSONUrl handles JSON URLs, it should instead import and use the SetupJSONUrl component.
| }, | ||
| "json": { | ||
| "fieldJson": "JSON", | ||
| "jsonUrlRequired": "Le JSON est obligatoire", |
There was a problem hiding this comment.
Bug: Incorrect JSON Translation Key Name
The French translation key settingsSetup.json.jsonUrlRequired is incorrectly named. It refers to a JSON content requirement, not a URL, which conflicts with its message "Le JSON est obligatoire" and how it's used in the code. It should be settingsSetup.json.jsonRequired.
|
@keabard hey, did you have time to look at the Cursor review? |
|
No sorry, didn't have the occasion to go through it yet. Yes a simple JSON import would do :) |
2 participants
It would be cool to avoid hosting our JSON or giving public access to an endpoint, and just paste a JSON object to configure a new OpenFeedback event.
This PR is meant to do that.
Could not manage to make the Firebase install step work tho, so this feature is not tested at all. I hope not much more is needed and that you will be able to complete it 🙏