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

fixed send notif delegate issue #1822

Merged
merged 6 commits into from
Aug 22, 2024
Merged

fixed send notif delegate issue #1822

merged 6 commits into from
Aug 22, 2024

Conversation

mishramonalisha76
Copy link
Contributor

Pull Request Template

Ticket Number

Description

  • Problem/Feature:

Type of Change

  • Bug fix
  • New feature
  • Code refactor
  • Documentation update
  • Other (please describe):

Checklist

  • Quick PR: Is this a quick PR? Can be approved before finishing a coffee.
    • Quick PR label added
  • Not Merge Ready: Is this PR dependent on some other PR/tasks and not ready to be merged right now.
    • DO NOT Merge PR label added

Frontend Guidelines

Build & Testing

  • No errors in the build terminal
  • Engineer has tested the changes on their local environment
  • Engineer has tested the changes on deploy preview

Screenshots/Video with Explanation

  • Before: Explain the previous behavior

  • After: What's changed now

Additional Context

Review & Approvals

  • Self-review completed
  • Code review by at least one other engineer
  • Documentation updates if applicable

Notes

Copy link

  • There is a typo in the import statement, useNavigate should be imported from 'react-router-dom'.
  • There is a typo in the variable name nagivate, it should be navigate.
  • The condition inside useEffect to navigate should check for the presence of alaisDetails instead of !alaisDetails.
  • There is a typo in the CSS property width="-webkit-fill-available", it should be width="100%".
  • In the css prop of the <Box> component, the css template literal is not being used for any styling, it can be removed.
  • In the condition userPushSDKInstance?.readmode(), readmode() seems to contain a typo, it should be readMode() or readmode.
import { FC, useEffect, useState } from 'react';
import { css } from 'styled-components';
import { useSelector } from 'react-redux';
import { useNavigate } from 'react-router-dom'; // Correct import

import { Box, Text } from 'blocks';
import { SendNotificationForm } from './components/SendNotificationForm';
import UnlockProfileWrapper, { UNLOCK_PROFILE_TYPE } from 'components/chat/unlockProfile/UnlockProfileWrapper';

import { UserStoreType } from 'types';
import { useGetAliasInfo, useGetChannelDetails } from 'queries';
import { useAccount } from 'hooks';
import { aliasChainIdToChainName } from 'helpers/UtilityHelper';
import { ALIAS_CHAIN } from '@pushprotocol/restapi/src/lib/config';

//add formik
//add condition for /send URL

const SendNotification: FC = () => {
  const [isAuthModalVisible, setIsAuthModalVisible] = useState(true);
  const { userPushSDKInstance } = useSelector((state: UserStoreType) => state.user);
  const { account, chainId } = useAccount();
  const { data: channelDetails } = useGetChannelDetails(account);
  const { data: aliasDetails } = useGetAliasInfo({
    alias: account,
    aliasChain: aliasChainIdToChainName[chainId as keyof typeof aliasChainIdToChainName] as ALIAS_CHAIN,
  });
  const { delegatees } = useSelector((state: any) => state.admin);
  const navigate = useNavigate(); // Corrected variable name

  useEffect(() => {
    if (!channelDetails && !delegatees?.length && !aliasDetails) navigate('/channels'); // Corrected condition
  }, [channelDetails]);

  return (
    <Box
      padding={{ dp: 'spacing-lg', ml: 'spacing-sm' }}
      display="flex"
      flexDirection="column"
      gap={{ dp: 'spacing-xl', ml: 'spacing-md' }}
      alignSelf="center"
      width={{ dp: '648px', ml: '357px' }}
      //this has to be responsive
      borderRadius="radius-lg"
      alignItems="center"
      backgroundColor="surface-primary"
      margin={{ dp: 'spacing-lg', ml: 'spacing-sm' }}
    >
      <Text
        color="text-primary"
        variant="h3-semibold"
        display={{ dp: 'block', ml: 'none' }}
      >
        Send Notification
      </Text>
      <Text
        color="text-primary"
        variant="h5-semibold"
        display={{ dp: 'none', ml: 'block' }}
      >
        Send Notification
      </Text>
      <Box width="100%">
        <SendNotificationForm channelDetails={channelDetails} />
      </Box>
      {isAuthModalVisible && userPushSDKInstance && userPushSDKInstance?.readMode() && ( // Corrected method name
        <Box
          display="flex"
          justifyContent="center"
          width="100%" // Corrected width value
          alignItems="center"
        >
          <UnlockProfileWrapper
            type={UNLOCK_PROFILE_TYPE.MODAL}
            showConnectModal={true}
            onClose={() => setIsAuthModalVisible(false)}
            description="Unlock your profile to proceed."
          />
        </Box>
      )}
    </Box>
  );
};

export { SendNotification };

All looks good.

Copy link

github-actions bot commented Aug 21, 2024

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-08-22 08:38 UTC

Copy link

In SendNotification.tsx:

  1. There is a typo in the variable name "nagivate", it should be "navigate".

  2. In the conditional rendering of the Text component, there is a typo in "Send Notification" which should be wrapped in curly braces.

  3. The logic inside the useEffect hook related to navigation seems correct. No issues found there.

  4. The CSS declaration within the Box component seems fine.

  5. The condition for displaying the UnlockProfileWrapper seems to be checking for the existence of userPushSDKInstance instead of checking its method "readmode". This might lead to a runtime error. You should check if userPushSDKInstance is not null before trying to access its methods.

  6. The semantic issue is the variable "nagivate" should be corrected to "navigate" for better clarity.

In Navigation.tsx:

  1. There is a missing closing parenthesis in the useEffect hook before the condition for onActiveNetwork.

  2. The logic within the useEffect hook for setting navigation setup seems reasonable.

  3. There are missing closing parentheses in the returnTransformedList function calls; these should be fixed.

  4. The syntax for the usage of Object.assign in constructing navList should be corrected. Each Object.assign should end with a comma.

  5. There are a few missing closing parentheses and semicolons within the code; you should ensure the syntax is correct to avoid any errors.

Overall, the logic and structure seem fine, but there are multiple typos and syntax errors that need to be corrected.

Copy link

File: src/modules/sendNotification/SendNotification.form.ts

  • There are missing closing brackets for the test functions in the yup schema.
  • In the getFormInitialValues function, the closing bracket after assigning the value for ctaLink is missing.

After rectifying the above issues, the code should look like this:

import { NotificationType } from '@pushprotocol/restapi';
import * as yup from 'yup';

import { SelectOption } from 'blocks/select/Select';
import { getRequiredFieldMessage } from 'common';
import { appConfig } from 'config';

export const getValidationSchema = (isSubsetRecipientPresent: boolean) => {
  return yup.object().shape({
    channelAddress: yup.string().required(getRequiredFieldMessage('Delegate')),
    chainId: yup.string().required(getRequiredFieldMessage('Chain')),
    type: yup.string().required(getRequiredFieldMessage('Type')),
    body: yup.string().required(getRequiredFieldMessage('Description')),
    setting: yup.string().required(getRequiredFieldMessage('Setting')),
    recipient: yup.string().test('recipient', getRequiredFieldMessage('Recipient'), function (value) {
      return (
        (this.parent.type !== 'SUBSET' || isSubsetRecipientPresent) && (this.parent.type !== 'TARGETTED' || !!value)
      );
    }),
    title: yup.string().test('title', getRequiredFieldMessage('Title'), function (value) {
      return !this.parent.titleChecked || !!value;
    }),
    mediaUrl: yup.string().test('mediaUrl', getRequiredFieldMessage('Media URL'), function (value) {
      return !this.parent.mediaUrlChecked || !!value;
    }),
    ctaLink: yup.string().test('ctaLink', getRequiredFieldMessage('CTA Link'), function (value) {
      return !this.parent.ctaLinkChecked || !!value;
    }),
  });
};

export const getFormInitialValues = (delegateesOptions: SelectOption[], aliasChainOptions: SelectOption[]) => {
  return {
    channelAddress: delegateesOptions[0]?.value || '',
    chainId: aliasChainOptions[0]?.value || appConfig.coreContractChain.toString(),
    type: 'BROADCAST' as NotificationType,
    recipient: '',
    titleChecked: false,
    mediaUrlChecked: false,
    ctaLinkChecked: false,
    title: '',
    body: '',
    setting: '0',
    mediaUrl: '',
    ctaLink: '',
  };
};

File: src/modules/sendNotification/SendNotification.tsx

All looks good.

File: src/modules/sendNotification/SendNotification.utils.tsx

  • The getChannelDelegatesOptions function is missing a closing bracket at the end of the function.
  • There are typos in the variable names: settingsOptions and defaultOption (these should be camelCased)

After the corrections, the code should look like this:

import { NotificationType } from '@pushprotocol/restapi';

import { Box } from 'blocks';
import { appConfig } from 'config';
import { convertAddrCaipToAddress, convertAddressToAddrCaip } from 'helpers/CaipHelper';
import { ChannelSetting } from 'helpers/channel/types';
import { ChannelDetails } from 'queries';

export const getChannelChainList = (channelDetails: ChannelDetails, account: string, onCoreNetwork: boolean) => {
  const aliases =
    channelDetails?.aliases
      ?.filter((alias) => alias.is_alias_verified && convertAddrCaipToAddress(alias.alias_address) === account)
      ?.map((alias) => parseInt(alias.alias_blockchain_id)) || [];
  if (onCoreNetwork) return [...aliases, appConfig.coreContractChain];
  else return aliases;
};

export const getChannelDelegatesOptions = (delegatees: [ChannelDetails]) => {
  if (delegatees && delegatees.length) {
    return delegatees?.map((channel) => ({
      icon: (
        <Box
          width="24px"
          height="24px"
          borderRadius="radius-xxs"
          overflow="hidden"
        >
          <img
            width="100%"
            height="100%"
            src={channel?.iconV2}
            alt={channel?.name}
          />
        </Box>
      ),
      label: channel?.name,
      value: channel?.channel,
    }));
  }
  return [];
};

export const getChannelSettingsOptions = (channelDetails: ChannelDetails) => {
  let settingsOptions = [];
  const defaultOption = { label: 'Default', value: '0' };
  if (channelDetails) {
    settingsOptions =
      JSON.parse(channelDetails?.channel_settings)?.map((settings: ChannelSetting, index: number) => ({
        label: settings?.description,
        value: (index + 1).toString(),
      })) || [];
  }
  return [...settingsOptions, defaultOption];
};

File: src/modules/sendNotification/components/SendNotificationForm.tsx

All looks good.

File: src/structure/Navigation.tsx

All looks good.

Copy link

File: src/modules/sendNotification/SendNotification.form.ts

  1. In the recipient test function, the logic should be updated to use strict equality (===) instead of truthy/falsy checks:
recipient: yup.string().test('recipient', getRequiredFieldMessage('Recipient'), function (value) {
  return (
    (this.parent.type !== 'SUBSET' || isSubsetRecipientPresent) && (this.parent.type !== 'TARGETTED' || value !== '')
  );
}),
  1. In the title test function, there is a missing closing parenthesis:
title: yup.string().test('title', getRequiredFieldMessage('Title'), function (value) {
  return !this.parent.titleChecked || value !== '';
}),
  1. In the mediaUrl test function, there is a missing closing parenthesis:
mediaUrl: yup.string().test('mediaUrl', getRequiredFieldMessage('Media URL'), function (value) {
  return !this.parent.mediaUrlChecked || value !== '';
}),
  1. In the ctaLink test function, there is a missing closing parenthesis:
ctaLink: yup.string().test('ctaLink', getRequiredFieldMessage('CTA Link'), function (value) {
  return !this.parent.ctaLinkChecked || value !== '';
}),

File: src/modules/sendNotification/SendNotification.tsx

  1. The comment //this has to be responsive is not a valid JavaScript comment and should be enclosed in a block comment /* comment */.

  2. In the line width="-webkit-fill-available", the use of -webkit-fill-available for width may have limited browser support and could cause cross-browser compatibility issues. It's recommended to use more widely supported CSS properties.

All looks good.

Copy link

File: src/modules/sendNotification/SendNotification.form.ts

  1. In the getValidationSchema function, there are missing closing parenthesis for the test functions for title, mediaUrl, and ctaLink.

Updated code:

return yup.object().shape({
    channelAddress: yup.string().required(getRequiredFieldMessage('Delegate')),
    chainId: yup.string().required(getRequiredFieldMessage('Chain')),
    type: yup.string().required(getRequiredFieldMessage('Type')),
    body: yup.string().required(getRequiredFieldMessage('Description')),
    setting: yup.string().required(getRequiredFieldMessage('Setting')),
    recipient: yup.string().test('recipient', getRequiredFieldMessage('Recipient'), function (value) {
      return (
        (this.parent.type !== 'SUBSET' || isSubsetRecipientPresent) && (this.parent.type !== 'TARGETTED' || !!value)
      );
    }),
    title: yup.string().test('title', getRequiredFieldMessage('Title'), function (value) {
      return !this.parent.titleChecked || !!value;
    }),
    mediaUrl: yup.string().test('mediaUrl', getRequiredFieldMessage('Media URL'), function (value) {
      return !this.parent.mediaUrlChecked || !!value;
    }),
    ctaLink: yup.string().test('ctaLink', getRequiredFieldMessage('CTA Link'), function (value) {
      return !this.parent.ctaLinkChecked || !!value;
    }),
});
  1. In the getValidationSchema function, there is a missing closing curly brace at the end of the function.

Updated code:

});

All looks good.

@rohitmalhotra1420 rohitmalhotra1420 merged commit a77f285 into main Aug 22, 2024
2 checks passed
@mishramonalisha76 mishramonalisha76 linked an issue Aug 22, 2024 that may be closed by this pull request
corlard3y pushed a commit that referenced this pull request Aug 23, 2024
* fixed send notif delegate issue

* Fixed the send notification button on the navigation for delegatee list

* added fixes for alias

* added fixes for alias

* fixed review comment

---------

Co-authored-by: abhishek-01k <[email protected]>
rohitmalhotra1420 added a commit that referenced this pull request Sep 30, 2024
* added loading colour

* add icon

* add components

* update styles

* update mobile view

* update titles

* change button labels

* update multiplier

* update useDailyRewards hook

* update unlock profile modal changes

* update usedailyrewards hook

* fix isLoading onclick

* update locked status/ rewards ui

* update new users status

* add skeleton loader

* comment out stake push activities

* update skeleton loader

* update locked icon

* format text

* feat: add alert ds component (#1781)

* feat: add alert ds component

* chore: update alert ds acc to review

* chore: update component acc to review

* update alert for error messages

* update indexes

* Update Dashboard.tsx

* remove unusued

* fixed review comments

* Update Button.tsx

* update completed state

* fixed the spinner and button

* fixed the spinner and button

* Added spinner component (#1783)

* added spinner component

* added review changes

* added correct variants

* added the review comments

* added the review comments

* Add progress bar to ds (#1787)

* feat: add progress bar to ds

* chore: update progress bar according to review

* add loading state

* update locked status

* update isLocked

* update loading state

* remove disabled state from button

* update locked divider

* update error message

* fix icons and illustrations

* update naming and imports

* update reward activites hook

* update type

* add types

* resolve conments

* update box

* update text changes

* update text

* update points real time

* update mutiplier conditions in bonus

* update gap

* update padding referal section

* adjust daily section ui

* update stake array

* update stake api

* update rewards verification hook

* fix: yield farming layouts and resize buttons to take full width (#1793)

* Add support for erc1155 (#1794)

* chore: add support for erc1155

* chore: update uiweb version to include erc1155 support

* Replaced older tab implementation in trending channels with the new tab block component (#1800)

* added

* moved function to utils

* tab changes

* reverted rewards tab change

* Update Dashboard.utils.ts

* Opt-In Dropdown for dashboard (#1802)

* Opt-In Dropdown for dashboard

* Fixed the comments and issues

* Fixed the context issue

* DS-Dapp Notification Component  (#1801)

* add in-app semantics and block files

* update notification component

* add localstorage identifier

* update library

* add duration option

* resolve comments

* update comment fixes

* fix comments

* testing notification

* update notification component

---------

Co-authored-by: rohitmalhotra1420 <[email protected]>

* DApp-1804 tag component added in blocks (#1805)

* tag in progress

* implemented tag component

* Tag component implemented in blocks

* update hook and export component

* update code

* add componentdid mount flow

* update mount logic

* fix stake font size

* fix conflicts

* Fixed create channel ui , fixed message for unlock profile, removed governance from sidebar (#1817)

* fixed minor ui issues

* fixed minor ui issues

* fixed minor ui issues

* fixed select text color

* fixed select text color

* fixed select text color

* fixed chain issue

* fixed send notif delegate issue (#1822)

* fixed send notif delegate issue

* Fixed the send notification button on the navigation for delegatee list

* added fixes for alias

* added fixes for alias

* fixed review comment

---------

Co-authored-by: abhishek-01k <[email protected]>

* DApp-1799 blocks/table (#1824)

* table in progress

* in progress

* in progress

* table complete

* fixed header route +  reverted changes

* added alignment support

* set debug to false

* fix comments

* export type and fix issues

* update rewards notification

* fix loading state

* add activit days

* update locked status

* updates test whitelist file for nft claims

* fixed smart contract for alpha nft

* update reset time

* refactor code

* update fixes

* stake push

* update types

* update yarn.lock

* update icons

* update icons batch 2

* sort stake section

* add local storage key

* update hasEnded and has text

* fixed channel notifications (#1852)

* added the new monotone icons (#1857)

* added support for icons in tag components (#1853)

* update stat in yield farming data store (#1859)

* update refetch leaderboard

* fix double text issue - media query

* update icon

* hide notifs

* update font sizes

* fix invalid verification proof issue

* remove console

* update success flow

* fixes done

* removed the png file

* lock file fixed

* remove isActive Account in auth hook

---------

Co-authored-by: Monalisha Mishra <[email protected]>
Co-authored-by: rohitmalhotra1420 <[email protected]>
Co-authored-by: Kalash Shah <[email protected]>
Co-authored-by: Monalisha Mishra <[email protected]>
Co-authored-by: Abhishek <[email protected]>
Co-authored-by: abhishek-01k <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants