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

feat: Add embeddable registration form #1026

Conversation

ahtesham-quraish
Copy link
Contributor

@ahtesham-quraish ahtesham-quraish commented Aug 8, 2023

Description:

Adding embedded registration form

VAN-1574

How Has This Been Tested?
It has been tested locally.

@ahtesham-quraish ahtesham-quraish requested a review from a team as a code owner August 8, 2023 12:03
@codecov
Copy link

codecov bot commented Aug 8, 2023

Codecov Report

Patch coverage: 96.55% and project coverage change: +0.70% 🎉

Comparison is base (d8b5653) 84.51% compared to head (247e6ab) 85.21%.

Additional details and impacted files
@@                       Coverage Diff                        @@
##           embeddable-registration-form    #1026      +/-   ##
================================================================
+ Coverage                         84.51%   85.21%   +0.70%     
================================================================
  Files                               113      114       +1     
  Lines                              2195     2326     +131     
  Branches                            631      664      +33     
================================================================
+ Hits                               1855     1982     +127     
- Misses                              324      328       +4     
  Partials                             16       16              
Files Changed Coverage Δ
src/common-components/RedirectLogistration.jsx 100.00% <ø> (ø)
src/register/RegistrationPage.jsx 96.21% <ø> (+0.21%) ⬆️
src/register/EmbeddableRegistrationPage.jsx 96.50% <96.50%> (ø)
src/register/data/utils.js 81.96% <100.00%> (+0.61%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ahtesham-quraish ahtesham-quraish force-pushed the ahtesham/VAN-1574/embedded-registration-form branch 8 times, most recently from d5d3fcd to 3dd17f9 Compare August 10, 2023 06:17
src/register/EmbedableRegistrationForm.jsx Outdated Show resolved Hide resolved
src/register/EmbedableRegistrationForm.jsx Outdated Show resolved Hide resolved
src/register/EmbedableRegistrationForm.jsx Outdated Show resolved Hide resolved
src/register/EmbedableRegistrationForm.jsx Outdated Show resolved Hide resolved
@ahtesham-quraish ahtesham-quraish force-pushed the ahtesham/VAN-1574/embedded-registration-form branch from 3dd17f9 to cac54f7 Compare August 15, 2023 07:11
@zainab-amir zainab-amir changed the title fix: WIP feat: Add embeddable registration form Aug 15, 2023
Copy link
Contributor

@syedsajjadkazmii syedsajjadkazmii left a comment

Choose a reason for hiding this comment

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

Although the coverage is increasing but if you see your file changes, there are alot of lines not covered by tests. please add (copy paste) their tests too.

src/MainApp.jsx Outdated Show resolved Hide resolved
src/register/EmbedableRegistrationForm.jsx Outdated Show resolved Hide resolved
src/register/EmbedableRegistrationForm.jsx Outdated Show resolved Hide resolved
src/register/EmbedableRegistrationForm.jsx Outdated Show resolved Hide resolved
src/register/EmbedableRegistrationForm.jsx Outdated Show resolved Hide resolved
src/register/EmbedableRegistrationForm.jsx Outdated Show resolved Hide resolved
src/register/EmbedableRegistrationForm.jsx Outdated Show resolved Hide resolved
src/register/index.js Outdated Show resolved Hide resolved
@ahtesham-quraish ahtesham-quraish force-pushed the ahtesham/VAN-1574/embedded-registration-form branch from cac54f7 to 8897992 Compare August 16, 2023 09:31
@zainab-amir zainab-amir changed the base branch from master to embeddable-registration-form August 17, 2023 06:11
@ahtesham-quraish ahtesham-quraish force-pushed the ahtesham/VAN-1574/embedded-registration-form branch 5 times, most recently from 68532fc to b02192e Compare August 17, 2023 10:42
Comment on lines 419 to 427

if (name === 'name') {
validateInput(
name,
value,
{ name: formFields.name, username: formFields.username, form_field_key: name },
!validationApiRateLimited,
false,
);
Copy link
Contributor

Choose a reason for hiding this comment

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

This complete functionality was added for embedded registration. We don't need it for registration page: https://github.com/openedx/frontend-app-authn/pull/950/files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed

Description:
Adding embedded registration form
VAN-1574
@ahtesham-quraish ahtesham-quraish force-pushed the ahtesham/VAN-1574/embedded-registration-form branch from b02192e to 247e6ab Compare August 21, 2023 06:45
@ahtesham-quraish ahtesham-quraish merged commit 90db7ba into embeddable-registration-form Aug 21, 2023
5 checks passed
@ahtesham-quraish ahtesham-quraish deleted the ahtesham/VAN-1574/embedded-registration-form branch August 21, 2023 07:08
syedsajjadkazmii pushed a commit that referenced this pull request Sep 7, 2023
Description:
Adding embedded registration form
VAN-1574
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants