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

DApp-1881 tags added rewards activity #1894

Merged

Conversation

rohitmalhotra1420
Copy link
Collaborator

@rohitmalhotra1420 rohitmalhotra1420 commented Oct 3, 2024

Pull Request Template

Ticket Number

Description

  • Problem/Feature:

  • Added support for tags in the Reward Activities component.

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

Screenshot 2024-10-03 at 3 08 36 PM

Additional Context

Review & Approvals

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

Notes

@rohitmalhotra1420 rohitmalhotra1420 added the Quick PR A PR that can be approved before finishing a coffee label Oct 3, 2024
@rohitmalhotra1420 rohitmalhotra1420 self-assigned this Oct 3, 2024
Copy link

github-actions bot commented Oct 3, 2024

  • In the getUpdatedExpiryTime function, the variable name days is misleading as it actually represents the day of the month. It would be clearer to name it dayOfMonth.

  • In the useEffect hook, there is a typo in the condition activity.activityType == 'follow_push_on_discord'. It should be === for comparison.

  • The condition in the useEffect hook should use logical OR || instead of logical AND && since we want to re-call handleLockStatus if activityType is either 'follow_push_on_discord' or 'follow_push_on_twitter'.

  • In the JSX, the Text component under the InfoFilled icon in the last Box has a variant h5-semibold, but there is a possibility that the h5 variant might not exist or have a different style than expected.

  • The innermost Box component inside another Box does not have specified width. It might cause layout issues depending on the content and screen size.

All looks good.

Copy link

github-actions bot commented Oct 3, 2024

PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-10-03 10:00 UTC

display="flex"
gap="spacing-xxs"
>
{!!activity.expiryType && (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
{!!activity.expiryType && (
{!!activity?.expiryType && (

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mishramonalisha76 expiryType will be always available for every activity.

Copy link

github-actions bot commented Oct 3, 2024

In RewardsActivitiesListItem.tsx:

  1. Typo in the prop padding={{ ml: 'spacing-xs', lp: 'spacing-sm spacing-xxs', initial: 'spacing-sm' }}, 'lp' should be 'pl'.
  2. Typo in the prop tag, it should be tag} and you are missing '}'. It should be <Lozenge size="small" icon={<Star />}>{tag}</Lozenge>.
  3. Typo in the prop varian, it should be 'variant'.
  4. Typo in the if condition: activity.activityType == 'follow_push_on_discord', double equals should be triple equals for strict equality check.
  5. Typo in the export statement, it should be export type RewardActivitiesListItemProps instead of export type RewardActivitiesListItemProps.

In rewards.ts:

  1. Typo in the definition of Prop type, 'type' missing in type Prop = {, should be type Prop = {
  2. Missing closing curly brace for export type UsersAllActivitiesResponse interface.
  3. Missing closing bracket for the Activity interface declaration, should be };
  4. Typo in the UsersActivity interface, it should be userId: string instead of userId: string.
  5. Typo in the RewardActivityStatusProps, should have a specific type of activities instead of any.

Corrected code:

RewardsActivitiesListItem.tsx:

padding={{ ml: 'spacing-xs', pl: 'spacing-sm spacing-xxs', initial: 'spacing-sm' }}
<Lozenge size="small" icon={<Star />}>{tag}</Lozenge>
variant="bs-regular"
if (activity.activityType === 'follow_push_on_discord' || activity.activityType === 'follow_push_on_twitter')
export type RewardActivitiesListItemProps

rewards.ts:
type Prop = {
export type UsersAllActivitiesResponse = {
  activities: UsersActivity[];
  total: number;
  page: number;
  size: number;
};
export type UsersActivity = {
  activityId: string;
  userId: string;
  activityTypeId: string;
  data: { twitter?: string; discord?: string };
  status: 'COMPLETED' | 'PENDING' | 'REJECTED';
  verificationProof: string;
  createdAt: string; // ISO 8601 date string
  updatedAt: string; // ISO 8601 date string
};
export type RewardActivityStatusProps = {
  activities: UsersActivity[];
};
Prop = {

All looks good.

@rohitmalhotra1420 rohitmalhotra1420 merged commit 96af1a3 into main Oct 3, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Quick PR A PR that can be approved before finishing a coffee
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants