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

Add test case for creating organization invoices #215

Merged
merged 7 commits into from
Mar 13, 2024

Conversation

SBMOYO
Copy link
Contributor

@SBMOYO SBMOYO commented Mar 11, 2024

Description

This pull request (PR) adds a new test case that covers scenarios related to invoice creation for organizations. It ensures that the process works correctly and checks if the organization variable is set to the team that created the invoice.

Motivation and Context

The motivation behind this change is to improve the accuracy of invoice creation within our system. By adding a specific test case and fixing the organization assignment, we ensure that invoices are associated correctly with the respective teams.

Dependencies

No specific dependencies are required for this change.

Checklist

  • Ran the Black Formatter and djLint-er on any new code.
  • Made necessary changes or additions to the documentation.
  • Ensured that changes generate no new warnings or errors.
  • Verified that new and existing unit tests pass locally with these changes.

What type of PR is this?

  • ✨ Feature (Test case addition)
  • 🐛 Bug Fix (Fixing organization assignment)

Added/updated tests?

  • 👍 yes

Related PRs, Issues

Copy link
Owner

@TreyWW TreyWW left a comment

Choose a reason for hiding this comment

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

Hi there @SBMOYO,

Thank you for the PR, really appreciate it. Can the ENV_DIR directory be removed from the PR though please since it is committing ~40 files we don't need.

Thanks

@SBMOYO
Copy link
Contributor Author

SBMOYO commented Mar 11, 2024

Hi @TreyWW
I have removed the ENV_DIR directory from the PR.

@TreyWW TreyWW self-requested a review March 11, 2024 12:26
@TreyWW
Copy link
Owner

TreyWW commented Mar 11, 2024

Hey @SBMOYO,

Did it definitely get removed, it still shows all files for me.

Try git rm -r -f --cached ENV_DIR maybe and then commit + push that with it deleted. On your branch the folder is still there too

@SBMOYO
Copy link
Contributor Author

SBMOYO commented Mar 11, 2024

how about now @TreyWW, Is it still there?

@TreyWW
Copy link
Owner

TreyWW commented Mar 11, 2024

That's better @SBMOYO,

Thanks, i'll give it a review now. Appreciate it :)

@SBMOYO
Copy link
Contributor Author

SBMOYO commented Mar 12, 2024

@TreyWW I noticed that you encountered an error while attempting to merge the branch. The issue seems to be related to invoice creation for an organization. Even when I explicitly set the organization value to a created team, the resulting invoice still has the organization variable set to None.

Here’s the relevant snippet from the test output:

self.assertEqual(invoice.organization, self.created_team)
# AssertionError: None != <Team: Team object (1)>

To address this, I set the organization as follows:

data["organization"] = self.created_team.pk

Could you please investigate this further? It appears that the organization assignment is not functioning as expected during invoice creation.

@TreyWW
Copy link
Owner

TreyWW commented Mar 12, 2024

Oh right yeah, apologies for that. I'll look into this in about 20 mins. Thanks for pointing it out

@TreyWW
Copy link
Owner

TreyWW commented Mar 12, 2024

Hi again @SBMOYO,

So the issue seemed to be with your login to team handler. The logged_in_as_team field never gets saved so the user isnt actually logged in.

   def login_to_team(self):
        self.login_user()
        self.log_in_user.logged_in_as_team = self.created_team
        # needs to save afterwards --> self.log_in_user.save()

Furthermore there's another bug,

AssertionError: 0 != 1 for self.assertEqual(len(invoices), 1)

this is due to invoices = Invoice.objects.filter(user=self.log_in_user) as it now needs to be invoices = Invoice.objects.filter(user=self.log_in_user.logged_in_as_team) as we set the invoice to an organisation.

@SBMOYO
Copy link
Contributor Author

SBMOYO commented Mar 13, 2024

@TreyWW Thanks for pointing out the issue with the login team handler.

Regarding the other bug:

AssertionError: 0 != 1 for self.assertEqual(len(invoices), 1)

If I update the code I like you suggested python invoices = Invoice.objects.filter(user=self.log_in_user.logged_in_as_team) the code raises a ValueError python ValueError: Cannot query "Team object (1)": Must be "User" instance

I have a question, when a user creates an invoice as an individual the organisation variable is set to None. What value is the user variable set to when a user creates an invoice for an organisation, is it set to None just like what happens to organisation?

@TreyWW
Copy link
Owner

TreyWW commented Mar 13, 2024

Sorry completely missed your message. Yep you're correct, if it's saving as an org then organisation is set to team object and user is none. And if setting under individual then user is the User object and organisation is none

@SBMOYO
Copy link
Contributor Author

SBMOYO commented Mar 13, 2024

Great, so that's where the problem was. I was testing if invoice.user and invoice.organisation had their values set to something else except None .

@TreyWW
Copy link
Owner

TreyWW commented Mar 13, 2024

Hi @SBMOYO,
Could you run black . again to format the files. Thanks

@TreyWW TreyWW self-requested a review March 13, 2024 20:15
@TreyWW TreyWW merged commit ab4bda7 into TreyWW:main Mar 13, 2024
14 checks passed
@TreyWW
Copy link
Owner

TreyWW commented Mar 13, 2024

Thanks for the PR @SBMOYO! Really appreciate it.

@SBMOYO SBMOYO deleted the feature/update_test_cases branch July 25, 2024 13:58
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.

[Enhance] Update test cases to work with both organizations + normal individual accounts
2 participants