Properties (#7) #8
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* new property app + clean up * added models for properties * Configuraion of menu (#4) * Added basics for the dashboard * Added detailed topbar and sidebar * Added middleware that allows user to work within context of a company * Added a function to return compay object for when of current company * added a mixin and decortor for views requiring user to have a company in the context * added ability to add an estate * Added ability to view estates * some base configs for managing properties * Added ability to add buildings * Added ability to add buildings * added ability to add units * Testing accounts (#5) * added testing for the User model * added test for user creation and authentication forms * added tests for Registration, login and logout * added middleware tests * set up coverage * S3 intergration and Image upload for properties (#6) * added ability to patch request. Now, users are able to update using patch requests * set up aws s3 with minio. Also added health storage check so that application can run with or without storage service * added new model for property images * added view and form mixins for image handling * added mixins to allow modular handling of uploading images * convert our toaster to use ES6 Modules * added a way to add images. Still need to configure to work accurately with our S3 server * fixed errors that prevent us from writing to out S3 * removed image uploading in creation of property. Images will be uploaded on update * fixed deletion of images and removed unnecessary logs * added readme, will finish later * Enhance user model with email verification features and update email settings - Added `email_verified` and `email_verification_token` fields to the User model for email verification. - Implemented `generate_verification_token` method to create a new verification token. - Updated email settings in `settings.py` to include configurable SMTP settings for email backend, host, port, and default sender email. - Cleaned up unused code in `patch-handler.js`. * Update myrealestate/properties/views.py Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> --------- Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Contributor
Reviewer's Guide by SourceryThis pull request implements a comprehensive property management system with features for managing estates, buildings, units, and images. The implementation includes a company-based access control system, S3/MinIO integration for image storage, and various UI components using DaisyUI. The changes also include test coverage setup and email verification features. ER diagram for Company and Property ModelserDiagram
COMPANY ||--o{ ESTATE : owns
COMPANY ||--o{ BUILDING : owns
COMPANY ||--o{ UNIT : owns
ESTATE ||--o{ BUILDING : contains
BUILDING ||--o{ UNIT : contains
UNIT ||--o{ SUBUNIT : contains
ESTATE {
int id
string name
string address
int total_buildings
json amenities
string estate_type
bool managing
}
BUILDING {
int id
string name
string building_type
string address
bool managing
}
UNIT {
int id
string number
string unit_type
bool is_vacant
decimal square_footage
int bedrooms
decimal bathrooms
bool furnished
date available_from
decimal base_rent
decimal deposit_amount
json features
}
SUBUNIT {
int id
string number
string subunit_type
bool furnished
date available_from
decimal base_rent
decimal deposit_amount
bool is_vacant
}
Class diagram for Property Management ModelsclassDiagram
class Estate {
+CharField name
+CharField address
+ForeignKey company
+IntegerField total_buildings
+JSONField amenities
+CharField estate_type
+BooleanField managing
+GenericRelation images
}
class Building {
+ForeignKey estate
+ForeignKey company
+CharField name
+CharField building_type
+CharField address
+BooleanField managing
+GenericRelation images
}
class Unit {
+ForeignKey building
+ForeignKey company
+CharField number
+CharField unit_type
+ForeignKey main_tenant
+BooleanField is_vacant
+DecimalField square_footage
+IntegerField bedrooms
+DecimalField bathrooms
+BooleanField furnished
+DateField available_from
+DecimalField base_rent
+DecimalField deposit_amount
+JSONField features
+GenericRelation images
}
class SubUnit {
+ForeignKey parent_unit
+CharField number
+CharField subunit_type
+ForeignKey sublet_tenant
+BooleanField furnished
+DateField available_from
+DecimalField base_rent
+DecimalField deposit_amount
+BooleanField is_vacant
+GenericRelation images
}
class PropertyImage {
+ForeignKey content_type
+PositiveIntegerField object_id
+GenericForeignKey property_object
+ImageField image
+CharField caption
+BooleanField is_primary
+PositiveIntegerField order
}
Estate --> Building : has many
Building --> Unit : has many
Unit --> SubUnit : has many
Estate --> PropertyImage : has many
Building --> PropertyImage : has many
Unit --> PropertyImage : has many
SubUnit --> PropertyImage : has many
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey @BaseKodu - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding more comprehensive docstrings to key classes and methods, particularly in the property models and image handling components.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟡 Testing: 3 issues found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
30
to
39
| def test_register_success(self): | ||
| """Test successful registration""" | ||
| response = self.client.post(self.register_url, self.valid_data) | ||
|
|
||
| # Check redirect to login page | ||
| self.assertRedirects(response, self.login_url) | ||
|
|
||
| # Check user was created | ||
| self.assertTrue(User.objects.filter(email=self.valid_data['email']).exists()) | ||
| user = User.objects.get(email=self.valid_data['email']) |
Contributor
There was a problem hiding this comment.
suggestion (testing): Add test for email verification token generation on registration
Since email verification features were added to the User model, we should test that the verification token is properly generated during registration and that email_verified is initially set to False.
Suggested implementation:
# Check user was created
self.assertTrue(User.objects.filter(email=self.valid_data['email']).exists())
user = User.objects.get(email=self.valid_data['email'])
# Check email verification status and token
self.assertFalse(user.email_verified)
self.assertIsNotNone(user.email_verification_token)
# Verify token is a valid UUID
try:
uuid.UUID(user.email_verification_token)
except ValueError:
self.fail("Email verification token is not a valid UUID")
You'll need to add the following import at the top of the file if it's not already present:
import uuid
Comment on lines
40
to
44
| def test_email_is_normalized(self): | ||
| """Test email normalization""" | ||
| email = '[email protected]' | ||
| user = User.objects.create_user(email=email, password='testpass123') | ||
| self.assertEqual(user.email, '[email protected]') No newline at end of file |
Contributor
There was a problem hiding this comment.
issue (testing): Add tests for email verification token methods
The User model has new email verification features but there are no tests for generate_verification_token() method or the email_verified field behavior.
| context = company_context(request) | ||
|
|
||
| self.assertIsNotNone(context['current_company']) | ||
| self.assertEqual(context['current_company'].id, self.company.id) |
Contributor
There was a problem hiding this comment.
suggestion (testing): Add test for company session data cleanup
Consider adding a test to verify that company session data is properly cleaned up when a user's access to a company is revoked or when a company is deleted.
Suggested implementation:
self.assertEqual(context['current_company'].id, self.company.id)
def test_context_processor_revoked_access(self):
"""Test context processor when user access to company is revoked"""
request = RequestFactory().get('/')
request.user = self.user
# Set company data before revoking access
company_data = company_dict(self.company, self.user)
request.company = company_data
# Revoke user's access to company
self.user.companies.remove(self.company)
context = company_context(request)
# Session data should be cleaned up
self.assertIsNone(context['current_company'])
self.assertEqual(len(context['companies']), 0)
def test_context_processor_deleted_company(self):
"""Test context processor when company is deleted"""
request = RequestFactory().get('/')
request.user = self.user
# Set company data before deletion
company_data = company_dict(self.company, self.user)
request.company = company_data
# Delete the company
self.company.delete()
context = company_context(request)
# Session data should be cleaned up
self.assertIsNone(context['current_company'])
self.assertEqual(len(context['companies']), 0)
Note: These tests assume that:
- The company_context processor handles cleanup of invalid session data
- The User model has a many-to-many relationship with Company through 'companies'
- The company_dict function exists and creates proper company session data
You may need to adjust the code if:
- The relationship between User and Company is structured differently
- The company deletion or access revocation is handled through different methods
- The session data structure is different in your application
Comment on lines
+14
to
+18
| ``` | ||
| python | ||
| python -m venv venv | ||
| source venv/bin/activate # On Windows: venv\Scripts\activate | ||
| ``` |
Contributor
There was a problem hiding this comment.
suggestion: Remove this superfluous line as it serves no purpose in the instructions
Suggested change
| ``` | |
| python | |
| python -m venv venv | |
| source venv/bin/activate # On Windows: venv\Scripts\activate | |
| ``` |
python -m venv venv
source venv/bin/activate # On Windows: venv\Scripts\activate
| ``` | ||
| ### Tailwind CSS Setup | ||
|
|
||
| 1. Install Tailwind dependencies: |
Contributor
There was a problem hiding this comment.
issue: The command for installing Tailwind dependencies is missing
Please add the specific command needed to install the Tailwind dependencies
Comment on lines
52
to
61
| current_company_id = request.session.get('current_company_id') | ||
| try: | ||
| company = companies.get(id=current_company_id) | ||
| except Company.DoesNotExist: | ||
| company = companies.first() | ||
|
|
||
| company_details = company_dict(company, user) | ||
| request.session['company'] = company_details | ||
| request.session['current_company_id'] = company.id | ||
| return company_details |
Contributor
There was a problem hiding this comment.
issue (code-quality): Extract code out into method (extract-method)
Comment on lines
113
to
115
| if self.estate: | ||
| return f"{self.name} in {self.estate.name}" | ||
| return self.name |
Contributor
There was a problem hiding this comment.
suggestion (code-quality): We've found these issues:
- Lift code into else after jump in control flow (
reintroduce-else) - Replace if statement with if expression (
assign-if-exp)
Suggested change
| if self.estate: | |
| return f"{self.name} in {self.estate.name}" | |
| return self.name | |
| return f"{self.name} in {self.estate.name}" if self.estate else self.name |
Comment on lines
340
to
352
| """Ensure there's always a primary image if images exist""" | ||
| was_primary = self.is_primary | ||
| super().delete(*args, **kwargs) | ||
|
|
||
| if was_primary: | ||
| # Try to set another image as primary | ||
| remaining_image = PropertyImage.objects.filter( | ||
| content_type=self.content_type, | ||
| object_id=self.object_id | ||
| ).first() | ||
| if remaining_image: | ||
| remaining_image.is_primary = True | ||
| remaining_image.save() |
Contributor
There was a problem hiding this comment.
suggestion (code-quality): Use named expression to simplify assignment and conditional (use-named-expression)
Suggested change
| """Ensure there's always a primary image if images exist""" | |
| was_primary = self.is_primary | |
| super().delete(*args, **kwargs) | |
| if was_primary: | |
| # Try to set another image as primary | |
| remaining_image = PropertyImage.objects.filter( | |
| content_type=self.content_type, | |
| object_id=self.object_id | |
| ).first() | |
| if remaining_image: | |
| remaining_image.is_primary = True | |
| remaining_image.save() | |
| """Ensure there's always a primary image if images exist""" | |
| was_primary = self.is_primary | |
| super().delete(*args, **kwargs) | |
| if was_primary: | |
| if remaining_image := PropertyImage.objects.filter( | |
| content_type=self.content_type, object_id=self.object_id).first(): | |
| remaining_image.is_primary = True | |
| remaining_image.save() |
| self.object = form.save(commit=False) | ||
| self.object.company = self.get_company() | ||
| self.object.save() | ||
| messages.success(self.request, f"Building created successfully.") |
Contributor
There was a problem hiding this comment.
suggestion (code-quality): Replace f-string with no interpolated values with string (remove-redundant-fstring)
Suggested change
| messages.success(self.request, f"Building created successfully.") | |
| messages.success(self.request, "Building created successfully.") |
| self.object = form.save(commit=False) | ||
| self.object.company = self.get_company() | ||
| self.object.save() | ||
| messages.success(self.request, f"Unit created successfully.") |
Contributor
There was a problem hiding this comment.
suggestion (code-quality): Replace f-string with no interpolated values with string (remove-redundant-fstring)
Suggested change
| messages.success(self.request, f"Unit created successfully.") | |
| messages.success(self.request, "Unit created successfully.") |
BaseKodu
added a commit
that referenced
this pull request
Feb 19, 2025
* Properties (#7) (#8) * new property app + clean up * added models for properties * Configuraion of menu (#4) * Added basics for the dashboard * Added detailed topbar and sidebar * Added middleware that allows user to work within context of a company * Added a function to return compay object for when of current company * added a mixin and decortor for views requiring user to have a company in the context * added ability to add an estate * Added ability to view estates * some base configs for managing properties * Added ability to add buildings * Added ability to add buildings * added ability to add units * Testing accounts (#5) * added testing for the User model * added test for user creation and authentication forms * added tests for Registration, login and logout * added middleware tests * set up coverage * S3 intergration and Image upload for properties (#6) * added ability to patch request. Now, users are able to update using patch requests * set up aws s3 with minio. Also added health storage check so that application can run with or without storage service * added new model for property images * added view and form mixins for image handling * added mixins to allow modular handling of uploading images * convert our toaster to use ES6 Modules * added a way to add images. Still need to configure to work accurately with our S3 server * fixed errors that prevent us from writing to out S3 * removed image uploading in creation of property. Images will be uploaded on update * fixed deletion of images and removed unnecessary logs * added readme, will finish later * Enhance user model with email verification features and update email settings - Added `email_verified` and `email_verification_token` fields to the User model for email verification. - Implemented `generate_verification_token` method to create a new verification token. - Updated email settings in `settings.py` to include configurable SMTP settings for email backend, host, port, and default sender email. - Cleaned up unused code in `patch-handler.js`. * Update myrealestate/properties/views.py --------- Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> * Enhance user registration with email verification and update email templates - Added email verification functionality to the user registration process, including `email_verified` and `email_verification_token` fields in the User model. - Implemented email verification workflow with views for sending verification emails and verifying email addresses. - Created new email templates for verification emails and success messages. - Updated URL routing to include paths for email verification. - Added utility functions for rendering email templates with inline CSS. - Updated settings for email backend configuration. This commit improves user account security by ensuring email addresses are verified before granting access to the application. * Add Docker support with configuration files and environment settings (#9) - Introduced Docker support by adding a Dockerfile, docker-compose.yaml, and .dockerignore. - Configured the Dockerfile to set up a Python environment with necessary dependencies and Node.js. - Created a docker-compose.yaml file to define services for the Django application, PostgreSQL database, MinIO for storage, and Mailpit for email handling. - Updated requirements.txt to include psycopg2-binary for PostgreSQL support. - Modified Django settings to use environment variables for configuration, enhancing flexibility for different environments. - Ensured static and media files are properly managed within the Docker setup. * Add development commands documentation for Docker, Django, Tailwind, and database operations - Created a new `commands.md` file to document essential development commands for managing Docker containers, running Django management commands, handling package installations, and performing database operations. - Included commands for Tailwind CSS management, testing, coverage reporting, and production deployment. - Enhanced the README.md to provide a clearer setup guide for Docker and local environments, detailing prerequisites and steps for both installation methods. * Add company settings management with forms, views, and templates * Added models for usage in the django admin * Implement user invitation and registration features - Added `InvitedUserRegistrationForm` to handle user registration via invitations, removing unnecessary fields and managing user creation. - Introduced `CompleteRegistrationView` for processing registration with email verification tokens, including user login upon successful registration. - Created `UserInvitationForm` for inviting users to a company, including email validation and permission checks. - Developed `CompanyUsersView` to list users with their access levels and statuses, along with an invitation modal in the template. - Enhanced the user interface with a new template for managing company users and integrated JavaScript for handling user invitations. - Updated URL routing to include new views for user management and invitations. This commit enhances user management capabilities within the application, allowing for streamlined user invitations and registrations. * Fixed the invite user modal issues * Enhance user invitation and registration workflow - Added complete registration view and URL routing for invited users - Created email templates (HTML and plain text) for user invitation emails - Updated views with debug logging using icecream - Removed debug console logs from company users template - Prepared template for complete registration page This is incomplete and we still have to come back to ensure that users can be able to accept invitations. Will be done later * Add TODO comment for complete registration view - Added a TODO note to remind developers about ensuring proper redirection for invited users completing their registration - Enhances tracking of pending implementation details for the user invitation workflow * Update myrealestate/templates/partials/navigation/topbar.html fixed mistaken typo Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com> --------- Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
new property app + clean up
added models for properties
Configuraion of menu (Configuraion of menu #4)
Added basics for the dashboard
Added detailed topbar and sidebar
Added middleware that allows user to work within context of a company
Added a function to return compay object for when of current company
added a mixin and decortor for views requiring user to have a company in the context
added ability to add an estate
Added ability to view estates
some base configs for managing properties
Added ability to add buildings
Added ability to add buildings
added ability to add units
Testing accounts (Testing accounts #5)
added testing for the User model
added test for user creation and authentication forms
added tests for Registration, login and logout
added middleware tests
set up coverage
S3 intergration and Image upload for properties (S3 intergration and Image upload for properties #6)
added ability to patch request. Now, users are able to update using patch requests
set up aws s3 with minio. Also added health storage check so that application can run with or without storage service
added new model for property images
added view and form mixins for image handling
added mixins to allow modular handling of uploading images
convert our toaster to use ES6 Modules
added a way to add images. Still need to configure to work accurately with our S3 server
fixed errors that prevent us from writing to out S3
removed image uploading in creation of property. Images will be uploaded on update
fixed deletion of images and removed unnecessary logs
added readme, will finish later
Enhance user model with email verification features and update email settings
email_verifiedandemail_verification_tokenfields to the User model for email verification.generate_verification_tokenmethod to create a new verification token.settings.pyto include configurable SMTP settings for email backend, host, port, and default sender email.patch-handler.js.Summary by Sourcery
Introduce a property management application with features for managing estates, buildings, and units, including image handling capabilities. Enhance the user model with email verification and update the UI with a new navigation system. Add middleware for company context management and refactor components for modularity. Update build configurations for storage and add tests for new functionalities.
New Features:
Enhancements:
Build:
Documentation:
Tests: