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

Feature to deactivate a user #150

Merged
merged 28 commits into from
Jul 21, 2024

Conversation

joboy-dev
Copy link
Contributor

Description

The changes I have made are to successfully deactivate a user's account. Proper checks are made to ensure that the user account is successfully deactivated, and errors are properly handled

Related Issue (Link to issue ticket)

Link to issue

Motivation and Context

The change is required because users can decide to deactivate their account at any point in time while using the application. This change caters to that.

How Has This Been Tested?

I tested this change in Postman. I also tested in my development environment using pytest. I tested for the following scenarios:

  • Sucess in deactivating the account.
  • Missing field
  • Confirmation field set to false
  • No Authorization header leading to unauthorization
  • User already inactive

Screenshots (if appropriate - Postman, etc):

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

tests/v1/test_user_deactivation.py Outdated Show resolved Hide resolved
Comment on lines +18 to +21

if schema.confirmation == False:
raise HTTPException(400, 'Confirmation required to deactivate account')

Copy link
Contributor

Choose a reason for hiding this comment

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

A proper confirmation should be done by sending the user an email with a deactivation link

Boss @zxenonx what do you think?

Copy link
Contributor Author

@joboy-dev joboy-dev Jul 21, 2024

Choose a reason for hiding this comment

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

I had that before but the person working on adding the emailing template to the database doesn't have it ready so I was told to implement what I had, that a central service would be used for emailing
The endpoint to even send the email was available but the person too could not proceed without the one adding the templates to the database.

Also, the confirmation email would be sent after the account has been deactivated successfully.
@Dev-wonderful

Copy link
Contributor

@Dev-wonderful Dev-wonderful Jul 21, 2024

Choose a reason for hiding this comment

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

I was the one who said you should implement what you have. Anyway, I hope the confirmation would allow the user revert the deactivation with a time limit, since you have decided to deactivate successfully before sending a confirmation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay
Let me see what I can do about that as quickly as possible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Dev-wonderful I have implemented the changes

Comment on lines 12 to 25
test_db_name = 'test_fastapi_db'
test_db_pw = 'postgres'
SQLALCHEMY_DATABASE_URL = f"postgresql+psycopg2://postgres:{test_db_pw}@localhost:5432/{test_db_name}"
engine = create_engine(SQLALCHEMY_DATABASE_URL)
TestingSessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine)

def override_get_db():
db = TestingSessionLocal()
try:
yield db
finally:
db.close()

app.dependency_overrides[get_db] = override_get_db
Copy link
Contributor

Choose a reason for hiding this comment

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

Find out how to mock db calls

@Freeman-kuch Freeman-kuch merged commit e97ff1c into hngprojects:dev Jul 21, 2024
1 check passed
@joboy-dev joboy-dev requested a review from Dev-wonderful July 22, 2024 09:04
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