Skip to content

Commit

Permalink
fix(creatives): make sure selected creatives are correct for the ad s…
Browse files Browse the repository at this point in the history
…et (#1287)

During testing for another PR, I noticed that when adding or creating a
new creative, it adds to all ad sets. We should only add it to the one
we are editing.

---


https://github.com/user-attachments/assets/77bbb89a-036d-41d4-abec-ad6321dbd8f7
  • Loading branch information
IanKrieger authored Aug 1, 2024
1 parent 37a874b commit 5499fd1
Show file tree
Hide file tree
Showing 11 changed files with 67 additions and 32 deletions.
12 changes: 10 additions & 2 deletions src/components/Creatives/CreateCreativeButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ import {
CreateCreativeDocument,
} from "@/graphql-client/graphql";

export function CreateCreativeButton() {
interface Props {
index: number;
}

export function CreateCreativeButton({ index }: Props) {
const { values, setFieldValue } = useFormikContext<CampaignForm>();
const [, , isCreating] = useField<boolean>("isCreating");
const [, newMeta, newHelper] = useField<Creative>("newCreative");
Expand All @@ -29,7 +33,11 @@ export function CreateCreativeButton() {
values.adSets.forEach((adSet, idx) => {
void setFieldValue(`adSets.${idx}.creatives`, [
...adSet.creatives,
validCreativeFields(data.createCreative, advertiser.id, true),
validCreativeFields(
data.createCreative,
advertiser.id,
idx === index,
),
]);
});
isCreating.setValue(false);
Expand Down
5 changes: 2 additions & 3 deletions src/components/Creatives/CreativeForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -120,10 +120,9 @@ const CreativeTypeSpecificFields = ({
}: {
creativeType?: string;
}) => {
if (creativeType === "notification_all_v1")
return <NotificationAd useCustomButton />;
if (creativeType === "notification_all_v1") return <NotificationAd />;
if (creativeType === "inline_content_all_v1")
return <InlineContentAd useCustomButton alignPreview="row" />;
return <InlineContentAd alignPreview="row" />;

return null;
};
Expand Down
8 changes: 5 additions & 3 deletions src/components/Creatives/CreativeSelect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export function CreativeSelect(
options: Creative[];
useSelectedAdStyle?: boolean;
showState?: boolean;
index?: number;
index: number;
hideCreated?: boolean;
useButtonSelection?: boolean;
} & PropsWithChildren,
Expand Down Expand Up @@ -99,9 +99,11 @@ export function CreativeSelect(
sx={{ maxWidth: "200px", alignSelf: "end", marginTop: 2 }}
onClick={(e) => {
e.preventDefault();

const mapped = curr.map((c) => ({ ...c, included: true }));
values.adSets.forEach((adSet, idx) => {
const mapped = curr.map((c) => ({
...c,
included: idx === index,
}));
void setFieldValue(
`adSets.${idx}.creatives`,
_.uniqBy([...adSet.creatives, ...mapped], "id"),
Expand Down
7 changes: 5 additions & 2 deletions src/locales/en.po

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 5 additions & 3 deletions src/locales/es.po

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 5 additions & 3 deletions src/locales/pt.po

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion src/locales/test.po

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 10 additions & 4 deletions src/user/ads/AdsExistingAd.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ import { useLingui } from "@lingui/react";
import { useQuery } from "@apollo/client";
import { filterCreativesByCampaignFormat } from "@/user/ads/filterCreativesByCampaignFormat";

export function AdsExistingAd() {
interface Props {
index: number;
}

export function AdsExistingAd({ index }: Props) {
const { _: lingui } = useLingui();
const { isShowingAds, setIsShowingAds } = useContext(FormContext);
const { creatives } = useAdvertiserCreatives();
Expand All @@ -47,7 +51,9 @@ export function AdsExistingAd() {
["asc", "desc"],
) as CreativeFragment[];

const filtered = creativeOptionList.filter((c) => c.state === "active");
const filtered = creativeOptionList.filter(
(c) => c.state === "active" || c.state === "draft",
);
const excludeExisting = filtered.filter((e) => {
const associatedOptions = creatives ?? [];
return associatedOptions.find((ao) => ao.id === e.id) === undefined;
Expand Down Expand Up @@ -79,7 +85,7 @@ export function AdsExistingAd() {
<Typography variant="subtitle1" fontWeight={500}>
<Trans>
Ads are modular building blocks that can be paired with ad sets to
build unique combinations. Your previously approved ads will show
build unique combinations. Your previously created ads will show
here. Select by using the box next to the name. Use the
&quot;Complete selection&quot; button to finish.
</Trans>
Expand Down Expand Up @@ -123,8 +129,8 @@ export function AdsExistingAd() {
advertiserId: advertiser.id,
included: false,
}))}
index={index}
useSelectedAdStyle={false}
showState={false}
useButtonSelection
/>
)}
Expand Down
6 changes: 3 additions & 3 deletions src/user/ads/InlineContentAd.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ import { msg, Trans } from "@lingui/macro";

interface InlineAdProps {
name?: string;
useCustomButton?: boolean;
useContainer?: boolean;
alignPreview?: "column" | "row";
index?: number;
}

export function InlineContentAd(props: InlineAdProps) {
Expand Down Expand Up @@ -91,10 +91,10 @@ const InlineAdForm = (props: InlineAdProps) => {
/>
)}

{props.useCustomButton !== true && (
{props.index !== undefined && (
<Stack direction="row" justifyContent="space-between" mt={1}>
<div />
<CreateCreativeButton />
<CreateCreativeButton index={props.index} />
</Stack>
)}
</>
Expand Down
6 changes: 3 additions & 3 deletions src/user/ads/NotificationAd.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import { useLingui } from "@lingui/react";

interface NotificationAdProps {
name?: string;
useCustomButton?: boolean;
useContainer?: boolean;
index?: number;
}

export function NotificationAd(props: NotificationAdProps) {
Expand Down Expand Up @@ -85,10 +85,10 @@ const NotificationAdForm = (props: NotificationAdProps) => {
helperText={_(msg`Example - https://brave.com/brave-rewards/`)}
/>

{props.useCustomButton !== true && (
{props.index !== undefined && (
<Stack direction="row" justifyContent="space-between" mt={1}>
<div />
<CreateCreativeButton />
<CreateCreativeButton index={props.index} />
</Stack>
)}
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,22 +43,32 @@ export function AdSetAds({ index }: Props) {
<ShowAdsButton />
</CardContainer>

<AdsExistingAd />
<CampaignFormatSpecificModal format={values.format} />
<AdsExistingAd index={index} />
<CampaignFormatSpecificModal format={values.format} index={index} />
</>
);
}

function CampaignFormatSpecificModal(props: { format: CampaignFormat }) {
function CampaignFormatSpecificModal(props: {
format: CampaignFormat;
index: number;
}) {
const [, meta, helper] = useField<boolean>("isCreating");
const name = "newCreative";

let adComponent;
if (props.format === CampaignFormat.PushNotification)
adComponent = <NotificationAd name={name} useContainer={false} />;
adComponent = (
<NotificationAd name={name} useContainer={false} index={props.index} />
);
else if (props.format === CampaignFormat.NewsDisplayAd)
adComponent = (
<InlineContentAd name={name} useContainer={false} alignPreview="row" />
<InlineContentAd
name={name}
useContainer={false}
alignPreview="row"
index={props.index}
/>
);

return (
Expand Down

0 comments on commit 5499fd1

Please sign in to comment.