Add admin manage article feature #382
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Good to see your 1st PR. I will review it. By the way, can you make sure the Github action successfully passed? It seems like you have linting issue. You can configure your IDE so that you can apply linter on saving the files. By the way, when you run |
|
For quick fix, you can do for And push commit. |
|
@mistydqwq Please remove the unused code and comments to keep the code tidy and ready for review. Thank you! |
|
I haven't started code review yet, just run on my machine. The UI looks great!!
|
There was a problem hiding this comment.
There is a small bug for all the string fields in the form. For example, if the article's doi field is current null in database. When I type something in form's doi field and then delete them, and then click "Save changes", the doi field in DB for this article will be changed to "" (empty string).
To fix this, you can check out how I handle this kind of edge case in onboard/page.tsx. You can essentially use setValueAs field to fix this. For example:
<TextField.Root
className="w-full"
size="3"
placeholder="(Optional)"
{...register("url", {
setValueAs: (value) => {
if (value === "") {
// when the input is empty string, set to null to avoid url validation
return null;
}
return value;
},
})}
>
| public async getArticleByLink( | ||
| link: string | ||
| link: string, | ||
| fetchFromDigitalLibrary: boolean = true |
There was a problem hiding this comment.
| fetchFromDigitalLibrary: boolean = true | |
| useDigitalLibraryFallback: boolean = true |
I think the naming can be better. It's confusing to name like this since if you set fetchFromDigitalLibrary to true, it will first fetch from our DB and if not found, it will fetch from Digital Library. Therefore, I think naming fetchFromDigitalLibrary is not accurate.
Just provide one of my random idea, we can name it useDigitalLibraryFallback. @joannechen1223 any thought?
There was a problem hiding this comment.
I prefer not to use this parameter and create another method instead, just like what you commented out in getDbArticleByLink as the logic of the digital library here is relatively complicated. Adding another API will be more straightforward if you are not going to reuse the digital library related logic.
There was a problem hiding this comment.
Actually like what I commented out, at first I implemented a separated API to get article directly from the database, but our professor suggests me to combine the getDbArticleByLink with the original getArticleByLink because these two function are pretty similar and he doesn't want the API to explode at the beginning. Also make sense. So I changed the getArticleByLink function to the current version. May need to discuss.
There was a problem hiding this comment.
Thanks for your hard work! It was a great initiative. I left some comments related to the recnet-api design and development, summarized to two major perspectives here from my point of view.
- For getting article by link, if the requirement is NOT to integrate the digital library and only query from DB, I suggest having a separate API and method to handle it. (Feel free to change to the original method name if you have a better naming idea.)
- For updating article API, I suggest updating by article ID instead of the link. Some edge cases were provided in the comments.
If you have any questions, please let me know.
| public async getArticleByLink( | ||
| link: string | ||
| link: string, | ||
| fetchFromDigitalLibrary: boolean = true |
There was a problem hiding this comment.
I prefer not to use this parameter and create another method instead, just like what you commented out in getDbArticleByLink as the logic of the digital library here is relatively complicated. Adding another API will be more straightforward if you are not going to reuse the digital library related logic.
|
@swh00tw @joannechen1223 Thanks a lot for your review! I've already started to work on your comments, and fixed some problems locally. I will resolve the remaining problems these days. Btw I left some comments in the useDigitalLibrary or create a new api part, which might need to be discussed. |
Good idea, I've changed the month field to dropdown list. And for the second question, I actually intended to do it. Because this feature I think is mainly used to modify the article's entry. At first I just think if we changed the article's link, and the form doesn't disappear, the form's link will not be the same as the link in the search box, which could also be a little bit weird. But If you think it's better to keep the form after saving the changes, I can do it. |
[Feature] Add Admin Panel Article Management
Background
This PR adds a new Article Management feature to the admin panel, allowing administrators to search for articles in the database by URL and modify their fields.
Changes Introduced
Frontend Changes
article/management/folder underrecnet/src/app/admin/.article/management/page.tsxfor the main page.article/management/ArticleManagementForm.tsxfor the article editing form.AdminPanelNavto include the new Article Management section.src/serve/routers/article.ts:getArticleByLinklogic for fetching articles based on URL and using digital library or not .Backend Changes
adminUpdateArticleto update article fields in the database.getArticleByLinkAPI:boolean useDigitalLibrarydigitalLibraryservice.true(ensuring no impact on existing behavior).false: The API retrieves articles directly from the database without usingdigitalLibrary.libs/recnet-api-modelHow to Test
useDigitalLibraryparameter works as expected (retrieves articles correctly when set tofalse).