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#3 session badges #2

Merged
merged 9 commits into from
Oct 28, 2024
Merged

Feat#3 session badges #2

merged 9 commits into from
Oct 28, 2024

Conversation

LeonardoVieira1630
Copy link
Member

Pull request made to complete this task: https://github.com/orgs/blockful-io/projects/16/views/1?pane=issue&itemId=83822933&issue=blockful-io%7Cbackend-zucity%7C3

The context is that we will now have "session" in the contract. Each villager can create a session. Each session automatically creates two new badges that only the session host can attest.
To achieve this, the following items were added:

  1. A map to store sessions (_session)
  2. A function to create a session (createSession())
  3. A function to check if the attestation is a session_attestation (isHostOnlyAttestation)
  4. A function to check if the attester is the host (isAttesterHost)
  5. A utility function slice() to get the first part of the attestation title
  6. Tests to validate the functionality of the new code.

@LeonardoVieira1630 LeonardoVieira1630 added the enhancement New feature or request label Oct 23, 2024
@LeonardoVieira1630 LeonardoVieira1630 self-assigned this Oct 23, 2024
Copy link

@lgahdl lgahdl left a comment

Choose a reason for hiding this comment

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

Seems good!

Choose a reason for hiding this comment

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

Hey, Leo! If it makes sense I would like to make two suggestions:

  1. Emit an event when a session is created
    Reason: Make it easy to access on the blockchain side in case it is necessary; Easy to track

Resolver.sol

 event SessionCreated(
        bytes32 indexed sessionId,
        address indexed host,
        string title,
        uint256 startTime,
        uint256 endTime
    );
    
    //function createSession()
     emit SessionCreated(
            sessionId,
            msg.sender,
            sessionTitle,
            session.startTime,
            session.endTime
        );
  1. Add function to end or cancel a session and session status (like: Active, Ended, Cancelled)
    Reason: Keep track of the session status; Make the user finish the session despite the time

Like:

  enum SessionStatus { Active, Ended, Cancelled }
    
    struct Session {
        address host;
        string title;
        uint256 startTime;
        uint256 endTime;
        SessionStatus status; 
    }

Choose a reason for hiding this comment

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

These are optional suggestions, so feel free to disagree in case they do not make sense to you!
Also, good job with the testing coverage and code organization

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi Rascar, ty for the review brother.

1- Great catch related to the event bro! Makes our lives easier. I will implement it.
2- I think that the status adds a complexity that is not ideal. Look, to modify a status in the struct, it would be necessary to call a function and pay for it. The best approach is just keep looking for the endTIme. If someone wants to know if a session is still valid, just need to look if currentTime<endTime.

@LeonardoVieira1630 LeonardoVieira1630 merged commit 8e727e6 into main Oct 28, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants