-
Notifications
You must be signed in to change notification settings - Fork 117
feat: extend the profile fields with the already existing extended_profile_fields #1167
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
base: master
Are you sure you want to change the base?
Conversation
Thanks for the pull request, @bra-i-am! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. Where can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
5503f7a
to
177091e
Compare
@bra-i-am looks like there are some failing tests, can you have a look? Also, I've invited you to the triage team which will allow tests to run automatically. Please accept that invite. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1167 +/- ##
==========================================
+ Coverage 66.82% 68.85% +2.03%
==========================================
Files 51 55 +4
Lines 856 1021 +165
Branches 176 224 +48
==========================================
+ Hits 572 703 +131
- Misses 273 304 +31
- Partials 11 14 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
(Same as in frontend-app-account comment) @e0d, I tried to cover all the code lines required to achieve the codecov check; there are some more warnings, but many are in lines so specific that I didn't get them with my changes. Please let me know if covering the whole code is required or if having the codecov check is enough. Thank you so much! 🙏 |
@openedx/2u-infinity this is ready for you. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Overview
This PR extends profile field functionality by integrating the existing extended_profile_fields into the profile forms and state. Key changes include:
- Introducing a new ExtendedProfileFields component to dynamically render additional field types (checkbox, text, select).
- Adding corresponding tests and updating services, actions, reducers, selectors, and sagas to handle extended profile fields.
- Updating the ProfilePage to integrate and render the extended profile fields.
Reviewed Changes
File | Description |
---|---|
src/profile/forms/ExtendedProfileFields.jsx | New component to render additional profile fields based on type |
src/profile/forms/elements/*.test.jsx | New tests covering CheckboxField, TextField, and SelectField components |
src/profile/data/sagas.js | Added saga and tests for fetching extended profile fields |
src/profile/data/services.js | Updated service to process visibility extended profile fields |
src/profile/data/actions.js | New actions for extended profile fields |
src/profile/utils.js | Exported utilities used by the new fields component |
src/profile/data/reducers.js | Updated reducer to store extended profile fields |
src/profile/data/selectors.js | Updated selectors to select extended profile fields from state |
src/profile/ProfilePage.jsx | Integrated ExtendedProfileFields into the profile page |
src/profile/forms/elements/FormControls.jsx | Minor update handling new prop for controlling visibility select |
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/profile/forms/ExtendedProfileFields.jsx:86
- Replace 'PropTypes.unknown' with 'PropTypes.any' as 'unknown' is not a standard PropType, ensuring compatibility with React PropTypes.
default: PropTypes.unknown,
return { | ||
...processedData, | ||
visibilityExtendedProfile: visibilityExtendedProfile ? JSON.parse(data['visibility.extended_profile']) : {}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrap the JSON.parse call in a try/catch to gracefully handle potential parsing errors when 'data["visibility.extended_profile"]' contains invalid JSON data.
return { | |
...processedData, | |
visibilityExtendedProfile: visibilityExtendedProfile ? JSON.parse(data['visibility.extended_profile']) : {}, | |
let parsedVisibilityExtendedProfile = {}; | |
if (visibilityExtendedProfile) { | |
try { | |
parsedVisibilityExtendedProfile = JSON.parse(data['visibility.extended_profile']); | |
} catch (error) { | |
logError(error, 'Failed to parse visibility.extended_profile JSON'); | |
} | |
} | |
return { | |
...processedData, | |
visibilityExtendedProfile: parsedVisibilityExtendedProfile, |
Copilot uses AI. Check for mistakes.
@@ -31,10 +32,17 @@ export const editableFormModeSelector = createSelector( | |||
formIdSelector, | |||
currentlyEditingFieldSelector, | |||
(account, isAuthenticatedUserProfile, certificates, formId, currentlyEditingField) => { | |||
const [parentPropKey, fieldName] = formId.split('/'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a conditional check to ensure that formId contains '/' before splitting to prevent runtime errors when the expected format is not met.
const [parentPropKey, fieldName] = formId.split('/'); | |
let parentPropKey, fieldName; | |
if (formId.includes('/')) { | |
[parentPropKey, fieldName] = formId.split('/'); | |
} else { | |
parentPropKey = formId; | |
fieldName = ''; | |
} |
Copilot uses AI. Check for mistakes.
dd27152
to
eb5b126
Compare
(Just a copy & paste from the comment at frontend-app-account :D ✨ ) @awais-ansari @sundasnoreen12, I hope you are doing great! I've been considering that this merge request might encounter issues during the merge due to the incorporation of a new feature. However, I also believe it would serve as a good recovery from the features lost in the transition to the MFEs (I planted this solution motivated by https://discuss.openedx.org/t/custom-fields-in-profile-form/6116/2, indeed). In any case, I'm waiting for your comments to plan how I could proceed. Beforehand, thank you so much for your time! |
Thanks @bra-i-am! We really need this for our NAU installation. |
598e3c7
to
e394cb2
Compare
…etchThirdPartyAuthContext
Description
This PR aims to allow the extension of the default profile fields by using the
extended_profile_fields
feature with a custom form app like the exampleMain changes
extended_profile_field
settings attribute (it is only covered checkbox, text and select fields)mfe_context
API)fieldValue
s from theextended_profile
account prop to theextended_profile_fields
attributes since the data is fetchedHow Has This Been Tested?
Sumac
environmenttutor mounts add /path/to/cloned/mfe
tutor mounts add lms:/path/to/cloned/custom-form-app:/openedx/extra-deps/custom-form-app
and install it usingtutor dev exec lms pip install -e ../extra-deps/custom-form-app
Site Configurations > local.openedx.io:8000
:tutor dev restart
Screenshots/sandbox (optional):