clean up more TODOs

This commit is contained in:
Jake Potrebic 2021-05-16 20:24:25 -07:00
parent dedf251ee6
commit 5453910e03
No known key found for this signature in database
GPG Key ID: 7C58557EC9C421F8
10 changed files with 90 additions and 35 deletions

View File

@ -24,11 +24,13 @@ spring:
#logging:
# level:
# root: TRACE
#logging:
# level:
# io:
# papermc:
# hangar: DEBUG
logging:
level:
io:
papermc:
hangar:
config:
jackson: DEBUG
fake-user:
enabled: false

View File

@ -11,8 +11,8 @@
v-model.trim="form.name"
:label="$t('channel.modal.name')"
:autofocus="!form.name"
:loading="validateLoading"
:error-messages="nameErrorMessages"
:loading="loadings.name"
:error-messages="errorMsgs.name"
:rules="[
$util.$vc.require($t('channel.modal.name')),
$util.$vc.regex($t('channel.modal.name'), validations.project.channels.regex),
@ -20,7 +20,7 @@
]"
/>
<v-card-subtitle class="pa-0 text-center">{{ $t('channel.modal.color') }}</v-card-subtitle>
<v-item-group v-model="form.color" mandatory>
<v-item-group v-model="form.color">
<v-container>
<v-row v-for="(arr, arrIndex) in swatches" :key="arrIndex" justify="center">
<v-col v-for="(color, n) in arr" :key="n" class="flex-grow-0 flex-shrink-1 pa-2 px-1">
@ -43,6 +43,14 @@
</v-row>
</v-container>
</v-item-group>
<v-text-field
v-model="form.color"
:label="$t('channel.modal.color')"
:error-messages="errorMsgs.color"
:loading="loadings.color"
:rules="[$util.$vc.require($t('channel.modal.color'))]"
readonly
/>
<v-checkbox v-model="form.nonReviewed" :label="$t('channel.modal.reviewQueue')" />
</v-form>
</v-card-text>
@ -72,8 +80,16 @@ export default class ChannelModal extends HangarFormModal {
@Prop({ type: Number, required: true })
projectId!: number;
validateLoading = false;
nameErrorMessages: TranslateResult[] = [];
loadings = {
name: false,
color: false,
};
errorMsgs = {
name: [] as TranslateResult[],
color: [] as TranslateResult[],
};
colors: Color[] = [];
form: ProjectChannel = {
name: '',
@ -118,8 +134,8 @@ export default class ChannelModal extends HangarFormModal {
@Watch('form.name')
checkName(val: string) {
if (!val) return;
this.validateLoading = true;
this.nameErrorMessages = [];
this.loadings.name = true;
this.errorMsgs.name = [];
this.$api
.requestInternal('channels/checkName', true, 'get', {
projectId: this.projectId,
@ -130,10 +146,32 @@ export default class ChannelModal extends HangarFormModal {
if (!err.response?.data.isHangarApiException) {
return this.$util.handleRequestError(err);
}
this.nameErrorMessages.push(this.$t(err.response.data.message));
this.errorMsgs.name.push(this.$t(err.response.data.message));
})
.finally(() => {
this.validateLoading = false;
this.loadings.name = false;
});
}
@Watch('form.color')
checkColor(val: string) {
if (!val) return;
this.loadings.color = true;
this.errorMsgs.color = [];
this.$api
.requestInternal('channels/checkColor', true, 'get', {
projectId: this.projectId,
color: val,
existingColor: this.channel?.color,
})
.catch((err: AxiosError) => {
if (!err.response?.data.isHangarApiException) {
return this.$util.handleRequestError(err);
}
this.errorMsgs.color.push(this.$t(err.response.data.message));
})
.finally(() => {
this.loadings.color = false;
});
}

View File

@ -9,8 +9,5 @@ public class CacheConfig {
public static final String AUTHORS_CACHE = "AUTHORS_CACHE";
public static final String STAFF_CACHE = "STAFF_CACHE";
// TODO dont need these caches anymore
public static final String PENDING_VERSION_CACHE = "PENDING_VERSION_CACHE";
public static final String NEW_VERSION_CACHE = "NEW_VERSION_CACHE";
}

View File

@ -1,6 +1,7 @@
package io.papermc.hangar.controller.internal;
import io.papermc.hangar.HangarComponent;
import io.papermc.hangar.model.common.Color;
import io.papermc.hangar.model.common.NamedPermission;
import io.papermc.hangar.model.common.PermissionType;
import io.papermc.hangar.model.db.projects.ProjectTable;
@ -50,6 +51,13 @@ public class ChannelController extends HangarComponent {
channelService.checkName(projectId, name, existingName);
}
@GetMapping("/checkColor")
@ResponseStatus(HttpStatus.OK)
@PermissionRequired(type = PermissionType.PROJECT, perms = NamedPermission.EDIT_TAGS, args = "{#projectId}")
public void checkColor(@RequestParam long projectId, @RequestParam Color color, @RequestParam(required = false) Color existingColor) {
channelService.checkColor(projectId, color, existingColor);
}
@Anyone
@VisibilityRequired(type = Type.PROJECT, args = "{#author, #slug}")
@GetMapping(path = "/{author}/{slug}", produces = MediaType.APPLICATION_JSON_VALUE)

View File

@ -151,7 +151,10 @@ public interface ProjectsApiDAO {
" u.join_date," +
" u.locked," +
" array_agg(r.name) roles," +
" -1 as projectCount" + // TODO yes, I do think we need to query this. This is public API, and it'd be wrong to just not include it here.
" (SELECT count(*)" +
" FROM project_members_all pma" +
" WHERE pma.user_id = u.id" +
" ) AS project_count" +
" FROM projects p " +
" JOIN project_stars ps ON p.id = ps.project_id " +
" JOIN users u ON ps.user_id = u.id " +
@ -177,7 +180,10 @@ public interface ProjectsApiDAO {
" u.join_date," +
" u.locked," +
" array_agg(r.name) roles," +
" -1 as projectCount" + // TODO yes, I do think we need to query this. This is public API, and it'd be wrong to just not include it here.
" (SELECT count(*)" +
" FROM project_members_all pma" +
" WHERE pma.user_id = u.id" +
" ) AS project_count" +
" FROM projects p " +
" JOIN project_watchers pw ON p.id = pw.project_id " +
" JOIN users u ON pw.user_id = u.id " +

View File

@ -29,7 +29,6 @@ public class PermissionRequiredVoter extends HangarDecisionVoter<PermissionRequi
this.setAllowMultipleAttributes(true);
}
// TODO debug logging
@Override
public int vote(Authentication authentication, MethodInvocation methodInvocation, Set<PermissionRequiredAttribute> attributes) {
if (!(authentication instanceof HangarAuthenticationToken)) {
@ -42,6 +41,8 @@ public class PermissionRequiredVoter extends HangarDecisionVoter<PermissionRequi
throw new IllegalStateException("Bad annotation configuration");
}
Permission requiredPerm = Arrays.stream(attribute.getPermissions()).map(NamedPermission::getPermission).reduce(Permission::add).orElse(Permission.None);
logger.debug("Possible permissions: " + hangarAuthenticationToken.getPrincipal().getPossiblePermissions());
logger.debug("Required permissions: " + requiredPerm);
Permission currentPerm;
switch (attribute.getPermissionType()) {
case PROJECT:
@ -73,6 +74,7 @@ public class PermissionRequiredVoter extends HangarDecisionVoter<PermissionRequi
default:
currentPerm = Permission.None;
}
logger.debug("Current permissions: " + currentPerm);
if (hangarAuthenticationToken.getPrincipal().isAllowed(requiredPerm, currentPerm)) {
return ACCESS_GRANTED;
}

View File

@ -51,6 +51,6 @@ public class PlatformService extends HangarComponent {
toBeRemovedMap.put(platform, toBeRemoved);
toBeAddedMap.put(platform, toBeAdded);
});
// TODO logging
// TODO user action logging
}
}

View File

@ -84,7 +84,6 @@ public class SSOService {
return new String(Base64.getEncoder().encode(payload.getBytes(StandardCharsets.UTF_8)));
}
// TODO logging
public AuthUser authenticate(String payload, String sig) {
Map<String, String> decoded = decode(payload, sig);
String nonce = decoded.get("nonce");

View File

@ -16,6 +16,7 @@ import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import java.util.List;
import java.util.function.Function;
import java.util.stream.Collectors;
@Service
@ -30,14 +31,23 @@ public class ChannelService extends HangarComponent {
}
public void checkName(long projectId, String name, @Nullable String existingName) {
List<ProjectChannelTable> existingChannels = projectChannelsDAO.getProjectChannels(projectId);
if (existingChannels.stream().filter(ch -> !ch.getName().equals(existingName)).anyMatch(ch -> ch.getName().equalsIgnoreCase(name))) {
checkName(projectId, name, existingName, projectChannelsDAO::getProjectChannels);
}
public void checkColor(long projectId, Color color, @Nullable Color existingColor) {
checkColor(projectId, color, existingColor, projectChannelsDAO::getProjectChannels);
}
private void checkName(long projectId, String name, @Nullable String existingName, Function<Long, List<ProjectChannelTable>> channelTableFunction) {
if (channelTableFunction.apply(projectId).stream().filter(ch -> !ch.getName().equals(existingName)).anyMatch(ch -> ch.getName().equalsIgnoreCase(name))) {
throw new HangarApiException(HttpStatus.CONFLICT, "channel.modal.error.duplicateName");
}
}
private void checkName(List<ProjectChannelTable> existingChannels, String name) {
private void checkColor(long projectId, Color color, @Nullable Color existingColor, Function<Long, List<ProjectChannelTable>> channelTableFunction) {
if (channelTableFunction.apply(projectId).stream().filter(ch -> ch.getColor() != existingColor).anyMatch(ch -> ch.getColor() == color)) {
throw new HangarApiException(HttpStatus.CONFLICT, "channel.modal.error.duplicateColor");
}
}
private void validateChannel(String name, Color color, long projectId, boolean nonReviewed, List<ProjectChannelTable> existingChannels) {
@ -49,14 +59,8 @@ public class ChannelService extends HangarComponent {
throw new HangarApiException(HttpStatus.BAD_REQUEST, "channel.modal.error.maxChannels", config.projects.getMaxChannels());
}
// TODO do we need to enforce unique colors?
if (existingChannels.stream().anyMatch(ch -> ch.getColor() == color)) {
throw new HangarApiException(HttpStatus.CONFLICT, "channel.modal.error.duplicateColor");
}
if (existingChannels.stream().anyMatch(ch -> ch.getName().equalsIgnoreCase(name))) {
throw new HangarApiException(HttpStatus.CONFLICT, "channel.modal.error.duplicateName");
}
checkName(projectId, name, null, (ignored) -> existingChannels);
checkColor(projectId, color, null, (ignored) -> existingChannels);
}
@Transactional

View File

@ -88,7 +88,6 @@ public class VersionService extends HangarComponent {
if (projectVersionTables.stream().filter(pv -> pv.getVisibility() == Visibility.PUBLIC).count() <= 1 && pvt.getVisibility() == Visibility.PUBLIC) {
throw new HangarApiException("version.error.onlyOnePublic");
}
// TODO previously, if this was a recommended version, it would auto assign a new one, but I don't think that's a good idea. easy enough to just set another, no requirement saying there has to be a recommended version
Visibility oldVisibility = pvt.getVisibility();
projectVersionVisibilityService.changeVisibility(pvt, Visibility.SOFTDELETE, comment);