Add mapfix history on maps page #294

Merged
itzaname merged 2 commits from feature/mapfix-list into staging 2025-12-27 04:51:04 +00:00
2 changed files with 290 additions and 39 deletions

View File

@@ -4,7 +4,7 @@ import { useParams, useNavigate } from "react-router-dom";
import { useState, useEffect } from "react";
import { Link } from "react-router-dom";
import { Snackbar, Alert } from "@mui/material";
import { MapfixStatus, type MapfixInfo } from "@/app/ts/Mapfix";
import { MapfixStatus, type MapfixInfo, getMapfixStatusInfo } from "@/app/ts/Mapfix";
import LaunchIcon from '@mui/icons-material/Launch';
import { useAssetThumbnail } from "@/app/hooks/useThumbnails";
@@ -23,7 +23,11 @@ import {
Stack,
CardMedia,
Tooltip,
IconButton
IconButton,
List,
ListItem,
ListItemIcon,
Pagination
} from "@mui/material";
import NavigateNextIcon from "@mui/icons-material/NavigateNext";
import CalendarTodayIcon from "@mui/icons-material/CalendarToday";
@@ -33,6 +37,11 @@ import BugReportIcon from "@mui/icons-material/BugReport";
import ContentCopyIcon from "@mui/icons-material/ContentCopy";
import InsertDriveFileIcon from "@mui/icons-material/InsertDriveFile";
import DownloadIcon from '@mui/icons-material/Download';
import HistoryIcon from '@mui/icons-material/History';
import CheckCircleIcon from '@mui/icons-material/CheckCircle';
import CancelIcon from '@mui/icons-material/Cancel';
import BuildIcon from '@mui/icons-material/Build';
import PendingIcon from '@mui/icons-material/Pending';
import {hasRole, RolesConstants} from "@/app/ts/Roles";
import {useTitle} from "@/app/hooks/useTitle";
@@ -45,6 +54,7 @@ export default function MapDetails() {
const [copySuccess, setCopySuccess] = useState(false);
const [roles, setRoles] = useState(RolesConstants.Empty);
const [mapfixes, setMapfixes] = useState<MapfixInfo[]>([]);
const [fixesPage, setFixesPage] = useState(1);
useTitle(map ? `${map.DisplayName}` : 'Loading Map...');
@@ -111,9 +121,8 @@ export default function MapDetails() {
allMapfixes = allMapfixes.concat(data.Mapfixes);
page++;
} while (allMapfixes.length < total);
// Filter out rejected, uploading, uploaded (StatusID > 7)
const active = allMapfixes.filter((fix: MapfixInfo) => fix.StatusID <= MapfixStatus.Validated);
setMapfixes(active);
// Store all mapfixes for history display
setMapfixes(allMapfixes);
} catch {
setMapfixes([]);
}
@@ -154,6 +163,16 @@ export default function MapDetails() {
}
};
const getStatusIcon = (iconName: string) => {
switch (iconName) {
case "Build": return BuildIcon;
case "Pending": return PendingIcon;
case "CheckCircle": return CheckCircleIcon;
case "Cancel": return CancelIcon;
default: return PendingIcon;
}
};
const handleSubmitMapfix = () => {
navigate(`/maps/${mapId}/fix`);
};
@@ -324,7 +343,8 @@ export default function MapDetails() {
sx={{
borderRadius: 2,
overflow: 'hidden',
position: 'relative'
position: 'relative',
mb: 3
}}
>
<Box sx={{ position: 'relative', overflow: 'hidden' }}>
@@ -355,6 +375,231 @@ export default function MapDetails() {
/>
</Box>
</Paper>
{/* Mapfix Section - Active + History */}
{mapfixes.length > 0 && (() => {
const activeFix = mapfixes.find(fix => fix.StatusID !== MapfixStatus.Rejected && fix.StatusID !== MapfixStatus.Released);
Quaternions marked this conversation as resolved
Review

Mapfixes UnderConstruction should not be considered active. ChangesRequested is debatable, but I would consider "active" to mean "requires staff attention", i.e. attention from someone who is not the map fixer.

const activeFix = mapfixes.find(fix => fix.StatusID !== MapfixStatus.Rejected && fix.StatusID !== MapfixStatus.Released && fix.StatusID !== MapfixStatus.UnderConstruction && fix.StatusID !== MapfixStatus.ChangesRequested);
Mapfixes UnderConstruction should not be considered active. ChangesRequested is debatable, but I would consider "active" to mean "requires staff attention", i.e. attention from someone who is not the map fixer. ```js const activeFix = mapfixes.find(fix => fix.StatusID !== MapfixStatus.Rejected && fix.StatusID !== MapfixStatus.Released && fix.StatusID !== MapfixStatus.UnderConstruction && fix.StatusID !== MapfixStatus.ChangesRequested); ```
Review

So the existing implementation was incorrect?

So the existing implementation was incorrect?
Review

Yes, it was incorrect. Submitting another mapfix is blocked if there is an active mapfix. This is the list of statuses which are considered "active":

// limit mapfixes in the pipeline to one per target map
ActiveAcceptedMapfixStatuses = []model.MapfixStatus{
model.MapfixStatusReleasing,
model.MapfixStatusUploaded,
model.MapfixStatusUploading,
model.MapfixStatusValidated,
model.MapfixStatusValidating,
model.MapfixStatusAcceptedUnvalidated,
}

Note that this list does not include fixes that are Under Review (MapfixStatus.Submitted), so multiple mapfixes for the same map may be submitted at the same time, but not accepted at the same time.

I think it would be useful to include submitted mapfixes (MapfixStatus.Submitted) in this list in case the submitter is about to duplicate an effort that is already submitted.

Yes, it was incorrect. Submitting another mapfix is blocked if there is an active mapfix. This is the list of statuses which are considered "active": https://git.itzana.me/StrafesNET/maps-service/src/commit/ae006565d689a4829cde53a274869caf54e34323/pkg/web_api/mapfixes.go#L23-L31 Note that this list does not include fixes that are Under Review (MapfixStatus.Submitted), so multiple mapfixes for the same map may be submitted at the same time, but not accepted at the same time. I think it would be useful to include submitted mapfixes (MapfixStatus.Submitted) in this list in case the submitter is about to duplicate an effort that is already submitted.
Review

I can fix it later since it was already like that.

I can fix it later since it was already like that.
const releasedFixes = mapfixes.filter(fix => fix.StatusID === MapfixStatus.Released);
const hasContent = activeFix || releasedFixes.length > 0;
if (!hasContent) return null;
// Pagination for released fixes
const fixesPerPage = 5;
const totalPages = Math.ceil(releasedFixes.length / fixesPerPage);
const startIndex = (fixesPage - 1) * fixesPerPage;
const endIndex = startIndex + fixesPerPage;
const paginatedFixes = releasedFixes
.sort((a, b) => b.CreatedAt - a.CreatedAt)
.slice(startIndex, endIndex);
return (
<Paper elevation={3} sx={{ p: 3, borderRadius: 2 }}>
<Box sx={{ display: 'flex', alignItems: 'center', mb: 2 }}>
<HistoryIcon sx={{ mr: 1.5, color: 'primary.main', fontSize: 24 }} />
<Typography variant="h6" component="h2" sx={{ fontWeight: 'bold' }}>
Mapfixes
</Typography>
</Box>
<Divider sx={{ mb: 2 }} />
<List sx={{ width: '100%' }}>
{/* Active Mapfix - shown first with special styling */}
{activeFix && (
<Box key={activeFix.ID}>
<ListItem
component={Link}
to={`/mapfixes/${activeFix.ID}`}
sx={{
py: 2,
px: 2,
borderRadius: 1,
transition: 'all 0.2s',
backgroundColor: 'rgba(25, 118, 210, 0.08)',
borderLeft: '4px solid',
borderColor: 'primary.main',
mb: releasedFixes.length > 0 ? 2 : 0,
'&:hover': {
backgroundColor: 'rgba(25, 118, 210, 0.12)',
transform: 'translateX(4px)'
},
textDecoration: 'none',
color: 'inherit',
display: 'block'
}}
>
<Box sx={{ display: 'flex', alignItems: 'flex-start', gap: 2 }}>
<ListItemIcon sx={{ minWidth: 36, mt: 0.5 }}>
{(() => {
const statusInfo = getMapfixStatusInfo(activeFix.StatusID);
const StatusIcon = getStatusIcon(statusInfo.iconName);
return (
<StatusIcon
sx={{
fontSize: 24,
color: statusInfo.color === 'default' ? 'text.secondary' :
statusInfo.color === 'error' ? 'error.main' :
statusInfo.color === 'warning' ? 'warning.main' :
statusInfo.color === 'success' ? 'success.main' :
statusInfo.color === 'primary' ? 'primary.main' : 'info.main'
}}
/>
);
})()}
</ListItemIcon>
<Box sx={{ flex: 1, minWidth: 0 }}>
<Typography
variant="body1"
component="div"
sx={{
fontWeight: 'bold',
mb: 1,
overflow: 'hidden',
textOverflow: 'ellipsis',
whiteSpace: 'nowrap'
}}
>
{activeFix.Description}
</Typography>
<Box sx={{ display: 'flex', gap: 1, mb: 1, flexWrap: 'wrap', alignItems: 'center' }}>
<Chip
label="Active"
size="small"
color="primary"
sx={{ fontWeight: 'bold' }}
/>
<Chip
label={getMapfixStatusInfo(activeFix.StatusID).label}
size="small"
color={getMapfixStatusInfo(activeFix.StatusID).color as any}
sx={{ fontWeight: 'medium' }}
/>
</Box>
<Box sx={{ display: 'flex', gap: 2, flexWrap: 'wrap', color: 'text.secondary' }}>
<Box sx={{ display: 'flex', alignItems: 'center', gap: 0.5 }}>
<PersonIcon sx={{ fontSize: 16 }} />
<Typography variant="caption">
{activeFix.Creator}
</Typography>
</Box>
<Box sx={{ display: 'flex', alignItems: 'center', gap: 0.5 }}>
<CalendarTodayIcon sx={{ fontSize: 16 }} />
<Typography variant="caption">
{formatDate(activeFix.CreatedAt)}
</Typography>
</Box>
</Box>
</Box>
<LaunchIcon sx={{ color: 'primary.main', fontSize: 18, mt: 0.5, flexShrink: 0 }} />
</Box>
</ListItem>
</Box>
)}
{/* Released Fixes History */}
{releasedFixes.length > 0 && (
<>
{activeFix && (
<Box sx={{ mb: 2, mt: 2 }}>
<Divider>
<Chip label={`${releasedFixes.length} Previous Fix${releasedFixes.length !== 1 ? 'es' : ''}`} size="small" />
</Divider>
</Box>
)}
{paginatedFixes.map((fix, index) => {
const statusInfo = getMapfixStatusInfo(fix.StatusID);
const StatusIcon = getStatusIcon(statusInfo.iconName);
return (
<Box key={fix.ID}>
<ListItem
component={Link}
to={`/mapfixes/${fix.ID}`}
sx={{
py: 2,
px: 2,
borderRadius: 1,
transition: 'all 0.2s',
'&:hover': {
backgroundColor: 'action.hover',
transform: 'translateX(4px)'
},
textDecoration: 'none',
color: 'inherit',
display: 'block'
}}
>
<Box sx={{ display: 'flex', alignItems: 'flex-start', gap: 2 }}>
<ListItemIcon sx={{ minWidth: 36, mt: 0.5 }}>
<StatusIcon
sx={{
fontSize: 24,
color: 'success.main'
}}
/>
</ListItemIcon>
<Box sx={{ flex: 1, minWidth: 0 }}>
<Typography
variant="body1"
component="div"
sx={{
fontWeight: 'bold',
mb: 0.5,
overflow: 'hidden',
textOverflow: 'ellipsis',
whiteSpace: 'nowrap'
}}
>
{fix.Description}
</Typography>
<Box sx={{ display: 'flex', gap: 2, flexWrap: 'wrap', color: 'text.secondary' }}>
<Box sx={{ display: 'flex', alignItems: 'center', gap: 0.5 }}>
<PersonIcon sx={{ fontSize: 16 }} />
<Typography variant="caption">
{fix.Creator}
</Typography>
</Box>
<Box sx={{ display: 'flex', alignItems: 'center', gap: 0.5 }}>
<CalendarTodayIcon sx={{ fontSize: 16 }} />
<Typography variant="caption">
{formatDate(fix.CreatedAt)}
</Typography>
</Box>
</Box>
</Box>
<LaunchIcon sx={{ color: 'primary.main', fontSize: 18, mt: 0.5, flexShrink: 0 }} />
</Box>
</ListItem>
{index < paginatedFixes.length - 1 && <Divider sx={{ my: 1 }} />}
</Box>
);
})}
{/* Pagination */}
{totalPages > 1 && (
<Box sx={{ display: 'flex', justifyContent: 'center', mt: 3 }}>
<Pagination
count={totalPages}
page={fixesPage}
onChange={(_, page) => setFixesPage(page)}
color="primary"
size="medium"
/>
</Box>
)}
</>
)}
</List>
</Paper>
);
})()}
</Grid>
{/* Map Details Section */}
@@ -399,39 +644,6 @@ export default function MapDetails() {
</Tooltip>
</Box>
</Box>
{/* Active Mapfix in Map Details */}
{mapfixes.length > 0 && (() => {
const active = mapfixes.find(fix => fix.StatusID <= MapfixStatus.Validated);
const latest = mapfixes.reduce((a, b) => (a.CreatedAt > b.CreatedAt ? a : b));
const showFix = active || latest;
return (
<Box>
<Typography variant="subtitle2" color="text.secondary">
Active Mapfix
</Typography>
<Box sx={{ display: 'flex', alignItems: 'center' }}>
<Typography
variant="body2"
component={Link}
to={`/mapfixes/${showFix.ID}`}
sx={{
textDecoration: 'underline',
cursor: 'pointer',
color: 'primary.main',
display: 'flex',
alignItems: 'center',
gap: 0.5,
mt: 0.5
}}
>
{showFix.Description}
<LaunchIcon sx={{ fontSize: '1rem', ml: 0.5 }} />
</Typography>
</Box>
</Box>
);
})()}
</Stack>
</Paper>

View File

@@ -66,9 +66,48 @@ function MapfixStatusToString(mapfix_status: MapfixStatus): string {
}
}
interface MapfixStatusInfo {
label: string;
color: 'default' | 'error' | 'warning' | 'success' | 'primary' | 'info';
iconName: string;
}
function getMapfixStatusInfo(statusId: MapfixStatus): MapfixStatusInfo {
Quaternions marked this conversation as resolved
Review

Isn't this info duplicated for the status icon on the mapfix page?

Isn't this info duplicated for the status icon on the mapfix page?
Review

That's why it's in a central importable lib instead of embedded into the page. Hunting down all duplication of this is not in scope for the feature. In the future this can be used instead.

That's why it's in a central importable lib instead of embedded into the page. Hunting down all duplication of this is not in scope for the feature. In the future this can be used instead.
switch (statusId) {
case MapfixStatus.UnderConstruction:
return { label: "Under Construction", color: "default", iconName: "Build" };
case MapfixStatus.ChangesRequested:
return { label: "Changes Requested", color: "warning", iconName: "Pending" };
case MapfixStatus.Submitting:
return { label: "Submitting", color: "info", iconName: "Pending" };
case MapfixStatus.Submitted:
return { label: "Submitted", color: "info", iconName: "CheckCircle" };
case MapfixStatus.AcceptedUnvalidated:
return { label: "Accepted (Unvalidated)", color: "primary", iconName: "CheckCircle" };
case MapfixStatus.Validating:
return { label: "Validating", color: "info", iconName: "Pending" };
case MapfixStatus.Validated:
return { label: "Validated", color: "success", iconName: "CheckCircle" };
case MapfixStatus.Uploading:
return { label: "Uploading", color: "info", iconName: "Pending" };
case MapfixStatus.Uploaded:
return { label: "Uploaded", color: "success", iconName: "CheckCircle" };
case MapfixStatus.Rejected:
return { label: "Rejected", color: "error", iconName: "Cancel" };
case MapfixStatus.Released:
return { label: "Released", color: "success", iconName: "CheckCircle" };
case MapfixStatus.Releasing:
return { label: "Releasing", color: "info", iconName: "Pending" };
default:
return { label: "Unknown", color: "default", iconName: "Pending" };
}
}
export {
MapfixStatus,
MapfixStatusToString,
getMapfixStatusInfo,
type MapfixInfo,
type MapfixList,
type MapfixStatusInfo,
}