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

Feature Queue Dynamo conversion to Typescript, and number of fixes #2603

Merged
merged 14 commits into from
Feb 12, 2025

Conversation

KaelenProctor
Copy link
Contributor

Improvements and fixes

  1. Convert Feature Queue dynamo to Typescript
  2. Handle edge case in Admin Featured queue UI of no items in featured queue, thus no "Last rotated date"
  3. Fix Rotate featured queue confirmation model which was empty
  4. Fix adding and removing a new featured queue in the Admin portal
  5. Fix error messaging in the rotate featured queue validation cases
  6. Removed the Move action of feature queue. Since ordered by date it would require a lot of item changes to move one item to another placement, which would change the original dates added to the queue
  7. Fix error messaging in the Patron featured queue actions

Testing

After

Add cube to featured queue:
new-add-cube-to-queue

Remove cube from featured queue:
new-remove-cube-from-queue

Successful rotation of featured queue:

Queue state:
new-featured-cube-rotate1
Featured cubes showing on dashboard:
new-featured-cube-rotate2
Queue state after rotate:
new-featured-cube-rotate3
Featured cubes dashboard after rotate:
new-featured-cube-rotate4

Patron cannot replace a currently featured cube:
new-user-replace-in-queue-error-currently-featured

Patron can add to the featured queue:
new-user-add-cube-to-queue

Patron can replace their item in the featured queue, as it isn't currently featured:
new-user-replace-cube-in-queue

Patron can remove from featured queue:
new-user-remove-from-queue

@@ -16,7 +16,7 @@ import MainLayout from 'layouts/MainLayout';
interface FeaturedCubesQueuePageProps {
cubes: Cube[];
daysBetweenRotations: number;
lastRotation: Date;
lastRotation: number;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From the backend and frontend code, it is a number

@@ -202,11 +202,11 @@ router.get('/featuredcubes', ensureAdmin, async (req, res) => {

return render(req, res, 'FeaturedCubesQueuePage', {
cubes: sortedCubes,
lastRotation: featured[0].featuredOn,
lastRotation: featured.length > 0 ? featured[0].featuredOn : new Date(0).valueOf(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Edge case that probably would only affect local development. If nothing in queue then the last rotation is 1969

});
});

router.post('/featuredcubes/rotate', ensureAdmin, async (req, res) => {
router.get('/featuredcubes/rotate', ensureAdmin, async (req, res) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Align with the rotate confirmation modal

@@ -237,25 +237,6 @@ router.post('/featuredcubes/rotate', ensureAdmin, async (req, res) => {
return redirect(req, res, '/admin/featuredcubes');
});

router.post(
'/featuredcubes/setperiod',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nothing using this endpoint

@@ -765,7 +765,7 @@ router.get('/social', ensureAuth, async (req, res) => {
});

router.post('/queuefeatured', ensureAuth, async (req, res) => {
const redirectTo = '/user/account?nav=patreon';
const redirectTo = '/user/account?nav=patreon&tab=4';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Might as well go straight to the patron tab when the user is doing featured queue actions

@@ -796,10 +796,10 @@ router.post('/queuefeatured', ensureAuth, async (req, res) => {

try {
if (shouldUpdate) {
fq.replaceForUser(req.user.id, cube.id);
await fq.replaceForUser(req.user.id, cube.id);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without the awaits Errors thrown by the functions wouldn't be caught and so the frontend would say "success" even though the action failed

@@ -5,14 +5,24 @@ import { canBeFeatured } from './featuredQueueUtil';

async function rotateFeatured(queue) {
if (queue.length < 4) {
throw new Error(`Not enough cubes in queue to rotate (need 4, have ${queue.length})`);
return {
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 code calling rotateFeatured expects a response with messages + succcess to indicate pass/fail, not errors

@@ -21,15 +31,15 @@ async function rotateFeatured(queue) {
const owner1 = await Patron.getById(old1.owner);
const owner2 = await Patron.getById(old2.owner);

for (const [owner, cube] of [
for (const [owner, featuredItem] of [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rename for clarity, aligning the variable with the dynamo model being called

@KaelenProctor KaelenProctor marked this pull request as ready for review February 10, 2025 21:08
@dekkerglen dekkerglen merged commit 306b2c0 into dekkerglen:master Feb 12, 2025
1 check passed
@KaelenProctor KaelenProctor deleted the feature/dynamo-5 branch February 12, 2025 12:24
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.

2 participants