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

feat(ui): Display invite links on the staff page #251

Open
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

mtiller
Copy link

@mtiller mtiller commented Feb 3, 2022

This adds the invitation URL to the server response for invitations and
it then leverages that URL on the client. The hope here is to address
issue #247 and ideally just make it easier to use Meli without having
to have a working email server to send out invites.

Design note: I very deliberately pass a URL back from the server
instead of just the bare token. The reason is that if I pass back
a bare token then the UI will have to formulate the URL which
means duplicating that logic both client and server side which
I have a visceral dislike for (very bad code smell, IMHO). This
way, only the server needs to know how to construct the URLs
and it can unilaterally change the scheme if it wants with
zero impact on the client. Hypermedia for the win.

This adds the invitation URL to the server response for invitations and
it then leverages that URL on the client.  The hope here is to address
issue getmeli#247 and ideally just make it easier to use Meli without having
to have a working email server to send out invites.

Design note: I very deliberately pass a URL back from the server
instead of just the bare token.  The reason is that if I pass back
a bare token then the UI will have to formulate the URL which
means duplicating that logic both client and server side which
I have a visceral dislike for (very bad code smell, IMHO).  This
way, only the server needs to know how to construct the URLs
and it can unilaterally change the scheme if it wants with
_zero_ impact on the client.  Hypermedia for the win.
@gempain
Copy link
Contributor

gempain commented Feb 7, 2022

@mtiller thanks so much for the contribution ! Will release this tonight. I think we should update the docs as well to emphasize this. It seems that the lint stage of the pipeline isn't passing, could you take a look ? Running the command eslint --fix should fix things.

@gempain
Copy link
Contributor

gempain commented Feb 8, 2022

@mtiller I tried to modify your branch to save you time but it seems I can't. Happy to release a new beta with all your PRs once this one is merged 😄

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.

None yet

3 participants