-
Notifications
You must be signed in to change notification settings - Fork 1
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 support for Key value pair in AccessType #575
base: master
Are you sure you want to change the base?
Conversation
onClick={() => handleOptionClick(group.name, selectedSubscriptions[group.name])} | ||
> | ||
{selectedSubscriptions[group.name]?.duration_length} | ||
{selectedSubscriptions[group.name]?.duration_length === 1 | ||
? selectedSubscriptions[group.name]?.duration_unit.substring( | ||
0, | ||
selectedSubscriptions[group.name]?.duration_unit.length - 1 | ||
) | ||
: selectedSubscriptions[group.name]?.duration_unit} | ||
| ||
{selectedSubscriptions[group.name]?.price_cents / 100} | ||
{selectedSubscriptions[group.name]?.price_currency} |
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.
If the same set of statements are repeated to access properties, fundamentals like variables are to be used!
Please consider programming practices over copy paste..
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.
Done
config/publisher.yml
Outdated
@@ -22,8 +22,8 @@ publisher: | |||
redirect_Url: "<Domain>/api/auth/v1/oauth/token" | |||
callback_Url: "Callback Url" | |||
accesstypeConfig: | |||
key: "Aw4ujaqhpn8aVMT7yzQawSyZ" | |||
accessTypeBkIntegrationId: 455 | |||
key: "yTKoDn8R1d4AY651ZPvGDUq4" |
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.
switch back to stg creds, do not commit prod keys or domains
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.
This is for QA testing. On staging, subscription page doesn't work. It only works on prod.
After testing, will change it back.
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.
use BK!, do not commit
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.
okay
<div styleName="selected-option" onClick={() => handleOptionClick(group.name, groupName)}> | ||
{groupName.duration_length} | ||
{groupName.duration_length === 1 | ||
? groupName.duration_unit.substring(0, groupName.duration_unit.length - 1) |
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.
Dry! Again!
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.
Changed
styleName={`option ${selectedOption === plan ? "option-select" : ""}`} | ||
onClick={() => handleOptionClick(group.name, plan)} | ||
> | ||
{1 + index}) |
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.
huh?
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.
its a design change for showing like this 1)
{groupDurationLength === 1 | ||
? groupDurationUnit.substring(0, groupDurationUnit.length - 1) | ||
: groupDurationUnit} | ||
|
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.
remove across the file, let's use CSS to achieve it
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.
okay
{plan.price_currency} | ||
{planCustomAttributes && | ||
planCustomAttributes.map((attribute, index) => ( | ||
<div key={index}>{attribute.value && renderRichText(attribute.value)}</div> |
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.
index shouldnt be a key
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.
removed
config/publisher.yml
Outdated
@@ -22,8 +22,8 @@ publisher: | |||
redirect_Url: "<Domain>/api/auth/v1/oauth/token" | |||
callback_Url: "Callback Url" | |||
accesstypeConfig: | |||
key: "Aw4ujaqhpn8aVMT7yzQawSyZ" | |||
accessTypeBkIntegrationId: 455 | |||
key: "yTKoDn8R1d4AY651ZPvGDUq4" |
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.
use BK!, do not commit
return ( | ||
<> | ||
<div | ||
key={index} |
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.
key shouldn't be index
)} | ||
<div> | ||
{group.subscription_plans.map((plan, index) => { | ||
const durationUnit = plan && plan.duration_unit; |
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.
plan will always be truthy
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.
destructure to a single line
Ref ticket: https://github.com/quintype/ace-planning/issues/861