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

feat: Store user space activty #127

Merged
merged 30 commits into from
Apr 16, 2024
Merged

feat: Store user space activty #127

merged 30 commits into from
Apr 16, 2024

Conversation

wa0x6e
Copy link
Contributor

@wa0x6e wa0x6e commented Aug 24, 2023

From https://www.notion.so/snapshotlabs/S-delegation-part-2-2b3186d8a33b4fd8b90c6575b6bf8000#bc7c1c9c785c46bdad69b4c8337771ca and snapshot-labs/snapshot-hub#653

  • Update the votes_count on new votes
  • Update the proposals_count on new proposal
  • Update the proposals_count on proposal deletion
  • Update the votes_count on proposal deletion
  • Update the proposals_count on space soft-deletion
  • Update the votes_count on space soft-deletion
  • Add a script to generate all counters

Use-cases not handled, due to them not being done through sequencer

  • Update the votes_count on space re-activation (space should be hard-deleted)
  • Update the proposals_count on space re-activation (space should be hard-deleted)
  • Update the votes_count on space migration (space migration should be deprecated)
  • Update the proposals_count on space migration (space migration should be deprecated)

@codecov
Copy link

codecov bot commented Aug 24, 2023

Codecov Report

Attention: Patch coverage is 31.48148% with 74 lines in your changes are missing coverage. Please review.

Files Patch % Lines
src/helpers/actions.ts 21.73% 54 Missing ⚠️
src/writer/delete-proposal.ts 31.57% 13 Missing ⚠️
src/writer/vote.ts 16.66% 5 Missing ⚠️
src/writer/delete-space.ts 75.00% 2 Missing ⚠️
Additional details and impacted files

📢 Thoughts on this report? Let us know!

src/writer/proposal.ts Outdated Show resolved Hide resolved
@wa0x6e wa0x6e marked this pull request as ready for review August 27, 2023 13:43
@wa0x6e wa0x6e requested a review from ChaituVR August 27, 2023 13:45
src/writer/vote.ts Outdated Show resolved Hide resolved
src/helpers/actions.ts Outdated Show resolved Hide resolved
Comment on lines 37 to 45
(async () => {
try {
await main();
process.exit(0);
} catch (e) {
console.error(e);
process.exit(1);
}
})();
Copy link
Member

@ChaituVR ChaituVR Aug 31, 2023

Choose a reason for hiding this comment

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

Do we want to run it every time the server starts? 🙈 We have 5 containers running. this may increase the load on the DB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this script is to be ran manually, only once to generate the counters

Copy link
Member

@ChaituVR ChaituVR Aug 31, 2023

Choose a reason for hiding this comment

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

There is no way to run manually on a single container I think 🤔 if we trigger it on DO's console will it continue to run even if we close browser? i didn't try it before

Copy link
Contributor Author

@wa0x6e wa0x6e Sep 1, 2023

Choose a reason for hiding this comment

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

Did not try either. In worst case, we leave the browser open until the task end, or we pipe the command to null (add > & at the end, which send the task to the background).

Ideally, we should handle that with a queue system (was proposed in a pitch in cycle 3)

test/schema.sql Outdated Show resolved Hide resolved
test/schema.sql Outdated Show resolved Hide resolved
src/helpers/actions.ts Outdated Show resolved Hide resolved
@wa0x6e
Copy link
Contributor Author

wa0x6e commented Apr 8, 2024

  • Branch update with master
  • Counter updates query has been merged together to be executed alongside the one inside the writers (since now, each insert is guaranteed to be a real insert, since the removal of IGNORE)

test/schema.sql Outdated Show resolved Hide resolved
Comment on lines 30 to 33
UPDATE user_space_activities
SET proposals_count = proposals_count - 1
WHERE user = ? AND space = ?
LIMIT 1;
Copy link
Member

Choose a reason for hiding this comment

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

What happens to votes_count of user

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Computed via refreshVotes below

Copy link
Member

@ChaituVR ChaituVR left a comment

Choose a reason for hiding this comment

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

tAck

@ChaituVR ChaituVR changed the title feat: update user_space counter feat: Store user space activty Apr 16, 2024
@wa0x6e wa0x6e merged commit 488df03 into master Apr 16, 2024
2 checks passed
@wa0x6e wa0x6e deleted the update-user-space-counters branch April 16, 2024 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants