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

refactor: activity generation #5464

Merged
merged 25 commits into from Dec 22, 2022
Merged

Conversation

MarkNerdi996
Copy link
Contributor

Summary

This PR aims to improve the activity generation.
One improvement is that now multiple outputs of one transaction are taken into account, therefore multiple activities can be generated.
Another improvement is, that we now have the possibility to insert logic for comparing an output with inputs. With this, we can later on add burning/consolidation/voting activities. This also implies that we now dont have to assume the activity action anymore, therefore all activities, for which we dont have the inputs, are classified as "unknown"

Changelog

- create multiple activities from 1 transaction
- add unknown activities if we dont know the inputs

Relevant Issues

Closes #5348

Testing

Platforms

Please select any platforms where your changes have been tested.

  • Desktop
    • MacOS
    • Linux
    • Windows
  • Mobile
    • iOS
    • Android

Instructions

Please describe the specific instructions, configurations, and/or test cases necessary to test and verify that your changes work as intended.

...

Checklist

Please tick all of the following boxes that are relevant to your changes.

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or modified tests that prove my changes work as intended
  • I have verified that new and existing unit tests pass locally with my changes
  • I have verified that my latest changes pass CI workflows for testing and linting
  • I have made corresponding changes to the documentation

@MarkNerdi996 MarkNerdi996 added type:enhancement Enhancement to existing feature type:refactor Refactor code context:wallet stardust Related to the Stardust Protocol labels Dec 21, 2022
@MarkNerdi996 MarkNerdi996 self-assigned this Dec 21, 2022
@kraftjs
Copy link
Contributor

kraftjs commented Dec 22, 2022

I've been testing this locally and my activity feed is wiped after ever logout. At first I thought I just needed to make new profiles because my old one's activity list was empty. I created a new profile and minted multiple nfts, which works and displays correctly in the activity list, but when I logged out and then back into the same profile my minted nfts were gone from the activity feed (though they still show in the collectibles tab).

Copy link
Contributor

@kraftjs kraftjs left a comment

Choose a reason for hiding this comment

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

Just a quick nitpick suggestion

packages/shared/locales/en.json Outdated Show resolved Hide resolved
MarkNerdi996 and others added 3 commits December 22, 2022 08:53
Co-authored-by: Jason Kraft <jason.kraft@iota.org>
Co-authored-by: Jason Kraft <jason.kraft@iota.org>
export function activityOutputContainsValue(wrappedOutput: IWrappedOutput): boolean {
const type = getActivityTypeFromOutput(wrappedOutput)
const typesToCheck = [ActivityType.Basic]
if (typesToCheck.includes(type)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is ActivityType.Basic assigned as a single element array and then searched with includes(type) when ActivityType.Basic === type would return the same? Will we eventually add more elements into typesToCheck?

@kraftjs
Copy link
Contributor

kraftjs commented Dec 22, 2022

Great work and LGTM 🔥 Glad we walked through this together this week because this one had lots of moving parts!

@kraftjs kraftjs merged commit 8228842 into stardust-develop Dec 22, 2022
@kraftjs kraftjs deleted the refactor/activity-generation branch December 22, 2022 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
context:wallet stardust Related to the Stardust Protocol type:enhancement Enhancement to existing feature type:refactor Refactor code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task]: Refactor activity generation to decide on multiple activities to generate
3 participants