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

Short URLs #465

Merged
merged 15 commits into from
Jun 12, 2024
Merged

Short URLs #465

merged 15 commits into from
Jun 12, 2024

Conversation

MelissaAutumn
Copy link
Member

@MelissaAutumn MelissaAutumn commented Jun 10, 2024

Fixes #369

This PR introduces schedule slugs and the ability to view availability using a schedule slug.

We'll phase out signed urls for the booking page eventually, and the user currently cannot customize a schedule slug, but this is a first step into making the "my link" small. I've also increased the space the "my link" textbox has by moving the copy button to a tooltip.

(Note: I have short links disabled on local dev, so on stage/prod it won't show the /user/ part.)
image

When you refresh your link, all schedule slugs will also be refreshed. Basically we shouldn't even see the booking signed urls anymore.

* Move generate slug to repo function
* Add migration to generate missing slugs
* Add function to retrieve signed url from slug
* Accidentally lint some more files
* Adjust naming of bookingView params
* If a user goes to bookingView with just scheduleOrSlug, it will look up the signed url and then request schedule from that
@MelissaAutumn MelissaAutumn self-assigned this Jun 10, 2024
backend/src/appointment/database/models.py Show resolved Hide resolved
return time_of_save.astimezone(zoneinfo.ZoneInfo(self.calendar.owner.timezone)).time()

@cached_property
def owner(self):
Copy link
Member Author

Choose a reason for hiding this comment

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

Convenience property

@@ -101,6 +101,16 @@ def refresh_signature(db: Session = Depends(get_db), subscriber: Subscriber = De
subscriber.id,
)

# Update schedule slugs as well!
# This is temp until we figure this flow out
schedules = repo.schedule.get_by_subscriber(db, subscriber.id)
Copy link
Member Author

Choose a reason for hiding this comment

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

Once we have multiple schedules we'll have to figure out a better flow for showing personal links and such. I assume we'll have one "General availability" schedule, but that's for future us!

frontend/src/views/BookingView.vue Outdated Show resolved Hide resolved
Copy link
Collaborator

@devmount devmount left a comment

Choose a reason for hiding this comment

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

Looking good so far!

backend/src/appointment/database/models.py Show resolved Hide resolved
return schedule.slug


def delete(db: Session, schedule_id: int):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to keep naming actual delete functions hard_delete?

* Always require namespacing by username for availability
* New function to lookup subscriber by schedule slug or signed url
* Add tooltips to text-buttons, they trigger on hover and focus.
* Add slug to schedule's test factory.
* Add tests for most auth dependencies.
@MelissaAutumn MelissaAutumn requested a review from devmount June 11, 2024 20:30
@MelissaAutumn
Copy link
Member Author

I still need to rebase the linting changes, but it's ready for review now.

@MelissaAutumn MelissaAutumn marked this pull request as ready for review June 11, 2024 20:42
Copy link
Collaborator

@devmount devmount left a comment

Choose a reason for hiding this comment

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

Edit: I first had issues with the migration, but running run-command main update-db solved it. Sorry for the hazzle 😅

Copy link
Collaborator

@devmount devmount left a comment

Choose a reason for hiding this comment

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

Works! Thanks!

@@ -1,13 +1,14 @@
"""Module: repo.subscriber

Repository providing CRUD functions for subscriber database models.
Repository providing CRUD functions for subscriber database models.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this a linter issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

Possibly my cat's doing 🤔

if len(match) > 1:
signature = match[1]
clean_url = clean_url.replace(signature, '')
username, signature, clean_url = utils.retrieve_user_url_data(url)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, thank you for moving that to utils

Comment on lines -278 to +306
send_confirmation_email, url=url, attendee_name=attendee.name, date=date, to=subscriber.preferred_email
send_confirmation_email, url=url, attendee_name=attendee.name, attendee_email=attendee.email, date=date,
to=subscriber.preferred_email
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not really related to this PR, but I found this during testing and decided to just fix it. Somehow we missed providing the attendee email which resulted in an error which was thrown on booking.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you!

@MelissaAutumn MelissaAutumn merged commit 0ccc949 into main Jun 12, 2024
4 checks passed
@MelissaAutumn MelissaAutumn deleted the features/369-real-short-urls branch June 12, 2024 16:40
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.

Restructure repository functions
2 participants