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

Bugfix - Settings not working for non-admin #104

Merged
merged 11 commits into from
Nov 21, 2023

Conversation

sandstromviktor
Copy link
Contributor

@sandstromviktor sandstromviktor commented Nov 17, 2023

Description

I found a small bug. When deploying a Develop app as a non-superuser, the option to change subdomain is disabled.
This causes the view logic in apps/views.py to try to replace a string with None, and it crashes.

How to spot the bug:

  1. Fire up serve from any other branch than this.
  2. Create a regular user
  3. Start a Jupyterlab instance
  4. Go to settings and click Update --> Here you get an error

This PR proposes a quick workaround for this.

I've also:

  1. fixed a previous mistake where i put the RESET_DB env variable in the wrong container in docker-compose.yaml
  2. Added a more strict if statement for the database reset.

Types of changes

What types of changes does your code introduce to Serve?

  • Hotfix (fixing a critical bug in production)
  • Bugfix
  • New feature
  • Documentation update

Checklist

If you're unsure about any of the items below, don't hesitate to ask. We're here to help!
This is simply a reminder of what we are going to look for before merging your code.

  • This pull request is against develop branch (not applicable for hotfixes)
  • I have included a link to the issue on GitHub or JIRA (if any)
  • I have included migration files (if there are changes to the model classes)
  • I have read the CONTRIBUTING doc
  • I have included tests to complement my changes
  • I have updated the related documentation (if necessary)
  • I have updated the release notes (docs/releasenotes.md)
  • I have added a reviewer for this pull request
  • I have added myself as an author for this pull request

@sandstromviktor sandstromviktor added the bug Something isn't working label Nov 17, 2023
@sandstromviktor sandstromviktor requested a review from a team November 17, 2023 12:01
@sandstromviktor sandstromviktor self-assigned this Nov 17, 2023
docker-compose.yaml Show resolved Hide resolved
apps/views.py Outdated Show resolved Hide resolved
@sandstromviktor sandstromviktor changed the title fixed bug where develop apps settings cant post if user not admin Bugfix - Settings not working for non-admin Nov 20, 2023
Copy link
Member

@alfredeen alfredeen left a comment

Choose a reason for hiding this comment

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

Nice work!

@sandstromviktor sandstromviktor merged commit 7650c2c into develop Nov 21, 2023
2 checks passed
@sandstromviktor sandstromviktor deleted the bugfix/settings-not-working-for-non-admin branch November 21, 2023 14:49
if new_release_name and current_release_name != new_release_name:
# This handles the case where a user creates a new subdomain, we must update the helm release aswell
new_url = appinstance.table_field["url"].replace(current_release_name, new_release_name)
appinstance.table_field.update({"url": new_url})
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this gets saved in the database. Might need to use save to make sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants