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

WIP: Allow joining public clans w/o invite token #362

Closed
wants to merge 3 commits into from

Conversation

Brutus5000
Copy link
Member

No description provided.

Copy link
Member

@bukajsytlos bukajsytlos left a comment

Choose a reason for hiding this comment

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

Very cool PR

src/main/java/com/faforever/api/clan/ClansController.java Outdated Show resolved Hide resolved
src/main/java/com/faforever/api/error/ErrorCode.java Outdated Show resolved Hide resolved
src/main/java/com/faforever/api/player/PlayerService.java Outdated Show resolved Hide resolved
@@ -17,7 +18,9 @@

private final PlayerRepository playerRepository;

public Player getPlayer(Authentication authentication) {
public Player getCurrentPlayer() {
Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure if UserSupplier could be used here, or it is being redundant due to this change

Copy link
Member Author

Choose a reason for hiding this comment

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

I wouldn't know a case where you have User instead of a Player except for the current logged in one.

@Brutus5000 Brutus5000 force-pushed the feature/public-clans branch 4 times, most recently from 78b3aec to 75de6d1 Compare November 13, 2019 23:26
produces = APPLICATION_JSON_UTF8_VALUE)
@ApiResponse(code = 200, message = "Success with JSON { jwtToken: ? }"),
@ApiResponse(code = 400, message = "Bad Request")})
@GetMapping(path = "/generateInvitationLink", produces = APPLICATION_JSON_VALUE)
Copy link

Choose a reason for hiding this comment

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

2 vs 4 spaces

@@ -47,6 +47,7 @@
private String description;
private String tagColor;
private String websiteUrl;
private Boolean requiresInvitation = Boolean.TRUE;
Copy link

Choose a reason for hiding this comment

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

Is that Boolean because of database definitions?

Copy link
Member

Choose a reason for hiding this comment

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

needs to be Boolean instead of boolean if it is nullable

@Brutus5000 Brutus5000 force-pushed the feature/public-clans branch 4 times, most recently from 81295e6 to a9c8c3a Compare July 5, 2020 21:15
@Brutus5000 Brutus5000 force-pushed the feature/public-clans branch 8 times, most recently from cf5ab3e to e345c1a Compare August 8, 2020 09:05
@Brutus5000 Brutus5000 force-pushed the feature/public-clans branch from e345c1a to 2879712 Compare August 19, 2020 20:32
@Brutus5000
Copy link
Member Author

Closed in favor of #428

@Brutus5000 Brutus5000 closed this Dec 5, 2020
@Brutus5000 Brutus5000 deleted the feature/public-clans branch June 10, 2024 20:28
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.

4 participants