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

Write Test For GetUserDropdownWorkspaces #1813

Conversation

MuhammadUmer44
Copy link
Contributor

@MuhammadUmer44 MuhammadUmer44 commented Jul 1, 2024

Describe your changes

image

Issue ticket number and link

Closes: #1805

Checklist before requesting a review

  • Bug fix (non-breaking change which fixes an issue)
  • I have performed a self-review of my code
  • I have tested on Chrome and Firefox
  • I have provided a screenshot or recording of changes in my PR if there were updates to the frontend

@MuhammadUmer44
Copy link
Contributor Author

Hi @elraphty, Please review this PR.


dbPerson := db.TestDB.GetPersonByUuid(person2.Uuid)

handlerUserHasAccess := func(pubKeyFromAuth string, uuid string, role string) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

@MuhammadUmer44 You should not mock this, the HasAccess Status should come from the controller/ business logic, mocking this defeats the purpose of this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@elraphty I check it

@elraphty
Copy link
Contributor

elraphty commented Jul 3, 2024

@MuhammadUmer44 what's the status of the requested change?

@MuhammadUmer44
Copy link
Contributor Author

Hi @elraphty, Please review this PR.

@@ -1394,6 +1394,20 @@ func (db database) GetPerson(id uint) Person {
return m
}

func (db database) DeleteWorkSpaceAllData() error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Put these functions in the db/test_config.go file, since we only need them for tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

db.TestDB.DeleteWorkSpaceAllData()

oHandler := NewWorkspaceHandler(db.TestDB)
oHandler.userHasAccess = db.TestDB.DeleteWorkSpaceUserAccessData
Copy link
Contributor

Choose a reason for hiding this comment

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

@MuhammadUmer44 This defeats the purpose, the function returns true it does not check if the user has access.

{WorkspaceUuid: workspace.Uuid, OwnerPubKey: person2.OwnerPubKey, Role: "ADD BOUNTY"},
{WorkspaceUuid: workspace.Uuid, OwnerPubKey: person2.OwnerPubKey, Role: "UPDATE BOUNTY"},
{WorkspaceUuid: workspace.Uuid, OwnerPubKey: person2.OwnerPubKey, Role: "DELETE BOUNTY"},
{WorkspaceUuid: workspace.Uuid, OwnerPubKey: person2.OwnerPubKey, Role: "PAY BOUNTY"},
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a "VIEW REPORT" role to the user, which will make the user have 5 roles, it will fix the userHasAccess error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@elraphty I have try but i got same error:
image

Copy link
Contributor

Choose a reason for hiding this comment

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

I dropped a comment

bountyCount := db.DB.GetWorkspaceBountyCount(uuid)
hasRole := db.UserHasAccess(pubkey, uuid, db.ViewReport)
bountyCount := oh.db.GetWorkspaceBountyCount(uuid)
hasRole := oh.userHasAccess(pubkey, uuid, db.ViewReport)
Copy link
Contributor

Choose a reason for hiding this comment

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

UserHasAccess is in the Database interface so you can change it to oh.db.UserHasAccess, let me know if this fixes it.

Copy link
Contributor Author

@MuhammadUmer44 MuhammadUmer44 Jul 3, 2024

Choose a reason for hiding this comment

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

@elraphty The same error still persists. I tried everything, but the UserHasAccess error is not fixed. Without verifying UserHasAccess, the unit test does not pass.

Copy link
Contributor

Choose a reason for hiding this comment

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

Push your latest changes, let me give it a look

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@MuhammadUmer44
Copy link
Contributor Author

@elraphty
Please check it:
image

@elraphty
Copy link
Contributor

elraphty commented Jul 4, 2024

@AbdulWahab3181 can you look into this?

@AbdulWahab3181
Copy link
Contributor

@elraphty sure, Let me take a look

@elraphty elraphty closed this Jul 4, 2024
@elraphty elraphty reopened this Jul 4, 2024
@MuhammadUmer44
Copy link
Contributor Author

HI @elraphty,
Please review it My unit test is Passed please check it:

image

@elraphty elraphty closed this Jul 4, 2024
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.

Write Test For GetUserDropdownWorkspaces
3 participants