Skip to content
This repository was archived by the owner on Jun 28, 2025. It is now read-only.

Feature/speaker and company profiles#171

Open
hugo2006alm wants to merge 39 commits intodevelopfrom
feature/speaker_and_company_profiles
Open

Feature/speaker and company profiles#171
hugo2006alm wants to merge 39 commits intodevelopfrom
feature/speaker_and_company_profiles

Conversation

@hugo2006alm
Copy link
Contributor

PR

This PR adds the capability for Companies and Speakers to have profiles of their own, using the base design of the Participant Profiles already included, with dynamic field loading according to type.

image
Participant
image
Speaker
image
Company

Note

I made this PR to have a little bit of feedback according the design choices (especially on the company one) aswell as to check if the info on the database for each type of Profile is enough or if I should add more info.

Also, can the companies and speakers edit their profiles or is that done through the Admin area when it is replaced/fixed??

Comment on lines +37 to +48
if (!user) {
response.notFound("Utilizador não encontrado")
return
}

const profile = User.getProfile(user)

if (!profile)
return response.redirect().toRoute('pages:signup')

return response.redirect().toRoute('pages:profile.show', { slug: user.participantProfile.slug } )
console.log(user.slug)
return response.redirect().toRoute('pages:profile.show', { slug: user.slug })
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't these be placed in service calls?

Copy link
Member

Choose a reason for hiding this comment

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

@Naaperas, redirects and similar stuff is okay to go in controllers. Business logic should go in services, though.

Hugo, please replace response.notFound with throw new HttpNotFoundException() (not sure if that's the correct name of the exception, but it's something like that). That way, the 404 page is shown to the user in that case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The exception must exist, and I've done it on the 404 branch, I'll merge it here when done. @limwa Could you check Slack to help me fix an error there? Thanks

Copy link
Collaborator

Choose a reason for hiding this comment

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

@limwa I have this commited on my local copy of this branch. Should I push or wait for the 404 branch?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ended up pushing this because of other reasons. See latest comment for updates on this.

@Naapperas Naapperas force-pushed the feature/speaker_and_company_profiles branch from 010a327 to af92557 Compare April 3, 2025 19:10
@Naapperas Naapperas force-pushed the feature/speaker_and_company_profiles branch from af92557 to 915a985 Compare April 3, 2025 19:14
@Naapperas
Copy link
Collaborator

I did my best trying to rebase this branch with the current state of develop. Since the PR included a lot of code, somethings might have gone unnoticed, @hugo2006alm please confirm that everything is in place.

@Naapperas
Copy link
Collaborator

@limwa @hugo2006alm I made a simple abstraction over the types of profiles. Currently there's a flag controlling whether to use the old UI or the new abstractions. I have left this flag only because I am not sure how to render staffs' profile pages (as they don't have the information that a normal profile might have). I could add the remaining information inside a new migration or leave it as is. What do you think?

Only included code for speakers, companies and participants. Other types of profiles are not yet implemented, such as staff profiles.
@Naapperas Naapperas requested a review from limwa April 4, 2025 01:53
@Naapperas
Copy link
Collaborator

I think a squash should be made to some of the commits in this PR. In that process, if we need to, I can delete the commit that introduced exceptions as part of the error handling logic. @limwa @hugo2006alm

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants