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

Quotes based on Specific Personality (#131) #324

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions frontend/src/components/Pages/Home/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { themes, animations, layouts, fonts, colorValues, quoteTypes } from '../
import TextField from '@material-ui/core/TextField';
import Autocomplete from '@material-ui/lab/Autocomplete';
import ContributorsCard from '../../ContributorsCard/ContributorCard'
import useQuoteAuthors from '../../../util/authors';
import { makeStyles } from '@material-ui/core/styles';

const useStyles = makeStyles({
Expand All @@ -24,6 +25,9 @@ const Home = () => {
const [bgColor, setBgColor] = useState(null);
const [borderColor, setBorderColor] = useState(null);
const [quoteType, setQuoteType] = useState("random");
const [quoteAuthor, setQuoteAuthor] = useState(null);

const { quoteAuthors, loadingQuoteAuthors } = useQuoteAuthors();

const classes = useStyles();

Expand Down Expand Up @@ -158,11 +162,25 @@ const Home = () => {
/>
</Grid>

<Grid item xs={12} sm={6} lg={3}>
<Autocomplete
id="author"
options={quoteAuthors}
value={quoteAuthor}
style={{ width: 300 }}
loading={loadingQuoteAuthors}
onChange={(_event, newValue) => {
setQuoteAuthor(newValue)
}}
renderInput={(params) => <TextField {...params} label="Author" placeholder="Select an author" variant="outlined" />}
/>
</Grid>
Comment on lines +165 to +177
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add error handling UI and improve accessibility.

The author selection implementation needs improvements in error handling and accessibility:

  1. Add error state UI from the useQuoteAuthors hook
  2. Add aria-label for better screen reader support
  3. Make margin styling consistent with other Autocomplete components

Apply these changes:

 <Grid item xs={12} sm={6} lg={3}>
+    {error && (
+      <Typography color="error" style={{ marginBottom: '10px' }}>
+        Failed to load authors: {error.message}
+      </Typography>
+    )}
     <Autocomplete
         id="author"
         options={quoteAuthors}
         value={quoteAuthor}
-        style={{ width: 300 }}
+        style={{ width: 300, margin: '0 auto' }}
         loading={loadingQuoteAuthors}
         onChange={(_event, newValue) => {
             setQuoteAuthor(newValue)
         }}
+        aria-label="Select quote author"
         renderInput={(params) => <TextField {...params} label="Author" placeholder="Select an author" variant="outlined" />}
     />
 </Grid>

Committable suggestion skipped: line range outside the PR's diff.


</Grid>

<Grid container spacing={4}>
<Grid item xs={12} style={{ marginTop: '20px' }}>
<TemplateCard theme={theme} animation={animation} layout={layout} font={font} fontColor={fontColor} bgColor={bgColor} borderColor={borderColor} quoteType={quoteType} />
<TemplateCard theme={theme} animation={animation} layout={layout} font={font} fontColor={fontColor} bgColor={bgColor} borderColor={borderColor} quoteType={quoteType} quoteAuthor={quoteAuthor}/>
</Grid>
<Grid item xs={12}>
<Typography align="center">Other layouts</Typography>
Expand All @@ -171,7 +189,7 @@ const Home = () => {
layouts.filter((item) => item !== layout).map((restLayout) => {
return (
<Grid key={restLayout} item xs={12} sm={12} md={6}>
<TemplateCard theme={theme} animation={animation} layout={restLayout} font={font} fontColor={fontColor} bgColor={bgColor} borderColor={borderColor} quoteType={quoteType} />
<TemplateCard theme={theme} animation={animation} layout={restLayout} font={font} fontColor={fontColor} bgColor={bgColor} borderColor={borderColor} quoteType={quoteType} quoteAuthor={quoteAuthor}/>
</Grid>
)
})
Expand Down
4 changes: 3 additions & 1 deletion frontend/src/components/organisms/TemplateCard/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@ const TemplateCard = (props) => {
const [snackbarMessage, setSnackbarMessage] = useState("");
const [isImageLoaded, setImageLoaded] = useState(false);
const originUrl = serverUrl; // Note: PORT 3004 since in server is served via that port. Frontend independently served on port 3000
const author = "Open Source";

const template = new Template();
const data = {
quote: "This is going to be the Github quote for your README",
author: "Open Source",
author: props.quoteAuthor ?? author,
};

const theme = { ...mainThemes[props.theme] };
Expand Down Expand Up @@ -78,6 +79,7 @@ const TemplateCard = (props) => {
...(props.bgColor && { bgColor: props.bgColor }),
...(props.fontColor && { fontColor: props.fontColor }),
...(isLayoutDefault && props.borderColor && { borderColor }),
...(props.quoteAuthor && { author: props.quoteAuthor }),
});
const quoteUrl = `${originUrl}/quote?${params.toString()}`;

Expand Down
39 changes: 39 additions & 0 deletions frontend/src/util/authors/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { useState, useCallback, useEffect } from "react";

import { serverUrl } from "../../components/Constants/urlConfig";

const useQuoteAuthors = () => {
const originUrl = serverUrl;
const [quoteAuthors, setQuoteAuthors] = useState([]);
const [loadingQuoteAuthors, setLoadingQuoteAuthors] = useState(false);

const fetchQuoteAuthors = useCallback(async () => {
setLoadingQuoteAuthors(true);
try {
const response = await fetch(`${originUrl}/authors`);
if (!response.ok) {
throw new Error(`HTTP error! status: ${response.status}`);
}

const data = await response.json();
if (data) {
setQuoteAuthors(data);
}
} catch (error) {
console.error("Failed to fetch quote authors:", error);
} finally {
setLoadingQuoteAuthors(false);
}
}, [originUrl]);

useEffect(() => {
fetchQuoteAuthors();
}, [fetchQuoteAuthors]);

return {
quoteAuthors,
loadingQuoteAuthors
};
}

export default useQuoteAuthors;
39 changes: 39 additions & 0 deletions src/api/controllers/authorsController.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
const quoteService = require("../services/quotesService");

const authorsController = async (req, res, next) => {

try {

let quotesUrl = req.query.quotesUrl || '';

let quoteCategory = req.query.quoteCategory || '';

let quoteObject = {
quotesUrl,
quoteCategory,
}

const authors = await quoteService.getAuthors(quoteObject);

res.setHeader("Content-Type", "application/json");

res.header(
"Cache-Control",
"no-cache,max-age=0,no-store,s-maxage=0,proxy-revalidate"
);
res.header("Pragma", "no-cache");
res.header("Expires", "-1");
res.json(authors);

} catch (error) {
console.error(error);
res.send({
name: error.name,
message: error.message,
});
}
};

module.exports = {
authorsController
};
3 changes: 3 additions & 0 deletions src/api/controllers/quotesController.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ const quoteController = async (req, res, next) => {

let quoteType = req.query.quoteType || '';

let author = req.query.author || undefined;

let quoteObject = {
theme,
animation,
Expand All @@ -42,6 +44,7 @@ const quoteController = async (req, res, next) => {
quoteCategory,
font,
quoteType,
author,
borderColor
}

Expand Down
4 changes: 4 additions & 0 deletions src/api/routes/quotes-router.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,16 @@


const controllers = require('../controllers/quotesController');
const authorsController = require('../controllers/authorsController');

const defineRoutes = (app) => {

// get a quote
app.get('/quote', controllers.quoteController);

// get authors
app.get('/authors', authorsController.authorsController);

Comment on lines +126 to +128
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add Swagger documentation for the /authors endpoint

While the route implementation looks good, it's missing Swagger documentation. This documentation is crucial for API consistency and client integration.

Add the following Swagger documentation above the route:

/**
 * @swagger
 * /authors:
 *   get:
 *     tags:
 *       - Authors
 *     description: Returns a list of quote authors
 *     parameters:
 *       - name: category
 *         in: query
 *         schema:
 *           $ref: "#/components/schemas/quoteCategory"
 *         description: Filter authors by quote category
 *     responses:
 *       200:
 *         description: List of authors
 *         content:
 *           application/json:
 *             schema:
 *               type: array
 *               items:
 *                 type: string
 *       500:
 *         description: Server error
 */

}

module.exports = defineRoutes;
76 changes: 68 additions & 8 deletions src/api/services/quotesService.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,16 @@ getQuoteIndex = (apiResponseLength, quoteType) => {
return (quoteType === "quote-for-the-day" ? epoch % apiResponseLength : Math.random() * apiResponseLength);
}

const filterQuotesByAuthor = (quotes, author) => {
const quotesFiltered = quotes.filter((quote) => quote?.author?.toString().toLowerCase().trim() === author.toString().toLowerCase().trim());

return quotesFiltered.length > 0 ? quotesFiltered : quotes;
}
marceloams marked this conversation as resolved.
Show resolved Hide resolved

const getQuote = async (quoteObj) => {

try {
let { theme, animation, layout, quotesUrl, quoteCategory, font, quoteType, borderColor } = quoteObj;
let { theme, animation, layout, quotesUrl, quoteCategory, font, quoteType, borderColor, author } = quoteObj;
let apiResponse;
let { customQuotesUrl, isValidUrl } = await getValidUrl(quotesUrl);
let isCustomQuote = false;
Expand All @@ -24,29 +30,43 @@ const getQuote = async (quoteObj) => {
//url from params is valid, proceed to verfiy the data
apiResponse = await requestApi(customQuotesUrl);

if (author) {
apiResponse = filterQuotesByAuthor(apiResponse, author);
}

marceloams marked this conversation as resolved.
Show resolved Hide resolved
if (apiResponse.length > 0) {
apiResponse = apiResponse[Math.floor(getQuoteIndex(apiResponse.length, quoteType))];
if (!apiResponse.quote && !apiResponse.author) {
apiResponse = await requestApi(url);
} else {

if (apiResponse.quote && apiResponse.author) {
isCustomQuote = true;
}
} else {
apiResponse = await requestApi(url);
}
}
else if (quoteCategory) {
apiResponse = quoteFromCategory[quoteCategory];

if (author) {
apiResponse = filterQuotesByAuthor(apiResponse, author);
}

apiResponse = apiResponse[Math.floor(getQuoteIndex(apiResponse.length, quoteType))];

isCustomQuote = true;
}
else {

if(!isCustomQuote) {
apiResponse = await requestApi(url);

if (author) {
apiResponse = filterQuotesByAuthor(apiResponse, author);
}

apiResponse = apiResponse[Math.floor(getQuoteIndex(apiResponse.length, quoteType))];
}

const template = new Template();
template.setTheme(theme);
template.setData(isCustomQuote ? apiResponse : apiResponse[Math.floor(getQuoteIndex(apiResponse.length, quoteType))]);
template.setData(apiResponse);
marceloams marked this conversation as resolved.
Show resolved Hide resolved
template.setFont(font);
template.setAnimation(animation);
template.setBorderColor(borderColor);
Expand All @@ -59,6 +79,46 @@ const getQuote = async (quoteObj) => {
}
};

const getAuthors = async (quoteObj) => {
try {
let {quotesUrl, quoteCategory} = quoteObj;
let apiResponse;
let { customQuotesUrl, isValidUrl } = await getValidUrl(quotesUrl);
let isCustomQuote = false;

if (isValidUrl) {
//url from params is valid, proceed to verfiy the data
apiResponse = await requestApi(customQuotesUrl);

if (apiResponse.length > 0) {
if (apiResponse[0].author) {
isCustomQuote = true;
}
}
}
else if (quoteCategory) {
apiResponse = quoteFromCategory[quoteCategory];
isCustomQuote = true;
}

if(!isCustomQuote) {
apiResponse = await requestApi(url);
}

if (!apiResponse || apiResponse.length === 0) {
return [];
}

// Get array of authors in alphabetical order and without duplicates
const authors = [...new Set(apiResponse.map(quote => quote.author).sort())];
marceloams marked this conversation as resolved.
Show resolved Hide resolved

return authors;
} catch (error) {
throw error;
}
}

module.exports = {
getQuote,
getAuthors
};