Create the UI for performing CRUD operations on the ISSUERS relation#856
Create the UI for performing CRUD operations on the ISSUERS relation#856olamidepeterojo wants to merge 2 commits intofedora-infra:swatantryafrom
Conversation
CRUD operations on the ISSUERS relation Signed-off-by: Olamide Ojo <peterojoolamide@gmail.com>
issuers relation Signed-off-by: Olamide Ojo <peterojoolamide@gmail.com>
|
Hello @gridhead, I have done the blog task as instructed and would like you take a look at this so i could get to working on implementing pagination for Issuers from endpoints if suitable by you |
gridhead
left a comment
There was a problem hiding this comment.
Please rework this pull request based on the suggested changes and review the AI-Assisted Contributions Policy and make necessary changes to disclose the use of AI tools. While we allow the use of AI tooling, please exercise discretion to ensure that you have moderation on its usage. This lets us see more of what you have to offer and less of what a certain AI tooling can. At the end of the day, we really want to hire human interns under our Outreachy mentorship and not AI toolings for the same.
| creationIssuer: builder.mutation({ | ||
| query: (issuerData) => ({ | ||
| url: "../api/admin/issuers", | ||
| method: "POST", | ||
| body: issuerData, | ||
| credentials: "include", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| }), | ||
| invalidatesTags: [], // optional for now | ||
| }), | ||
| updationIssuer: builder.mutation({ | ||
| query: ({ issuer_id, filldata }) => ({ | ||
| url: `../api/admin/issuers/${issuer_id}`, | ||
| method: "PUT", | ||
| body: filldata, | ||
| credentials: "include", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| }), | ||
| invalidatesTags: [], // optional for now | ||
| }), |
There was a problem hiding this comment.
Relying on the endpoints for your changes that would be introduced as a part of the solution for #695, that has not yet made their way into the codebase, is rather pretentious. 👎🏼
| <FloatingLabel controlId="issuerId" label="Issuer ID"> | ||
| <Form.Control | ||
| type="number" | ||
| value={form.id} | ||
| onChange={(e) => handleFormChange("id", e.target.value)} | ||
| placeholder="Issuer ID" | ||
| /> | ||
| </FloatingLabel> |
There was a problem hiding this comment.
I want you to take a step back and evaluate your suggested change. The issuer identity is something that is internal to the database, and hence, there is no way in which a user or an administrator can be expected to know about it. Have you tested these changes locally or simply gone ahead with opening a pull request with whatever an AI tool provided you with, without understanding what the codebase does?
| "Content-Type": "application/json", | ||
| }, | ||
| }), | ||
| invalidatesTags: [], // optional for now |
There was a problem hiding this comment.
What is the point of introducing an empty list under the invalidateTags attribute if it is not going to be used?
| "Content-Type": "application/json", | ||
| }, | ||
| }), | ||
| invalidatesTags: [], // optional for now |
There was a problem hiding this comment.
What is the point of introducing an empty list under the invalidateTags attribute if it is not going to be used?
| "Content-Type": "application/json", | ||
| }, | ||
| }), | ||
| invalidatesTags: [], // optional for now |
There was a problem hiding this comment.
Also, your pull request looks incomplete from your end only, since you have left the work-in-progress comment there, and yet you seem to have sent it over for reviews, without being respectful to both your and your mentors' time and efforts.
| }), | ||
| invalidatesTags: (result, error, { user_id }) => [{ type: "Identity", id: user_id }, "IdentitySearch"], | ||
| }), | ||
| creationIssuer: builder.mutation({ |
There was a problem hiding this comment.
I do not see a call for viewing/reading Issuers here.
There was a problem hiding this comment.
Why do we have backend-related changes when the issue ticket #681 clearly requests changes in the UI codebase? Even worse than that, why are these duplicates from your previous work from #855? If you are led to believe that duplication of work would somehow increase the likelihood of your selection in this programme, then you are unfortunately mistaken. It just makes us not want to work with you anymore, as these exhibited signs tell us that you are not respectful of the time and efforts put in at either end.
There was a problem hiding this comment.
Why do we have backend-related changes when the issue ticket #681 clearly requests changes in the UI codebase? Even worse than that, why are these duplicates from your previous work from #855? If you are led to believe that duplication of work would somehow increase the likelihood of your selection in this programme, then you are unfortunately mistaken. It just makes us not want to work with you anymore, as these exhibited signs tell us that you are not respectful of the time and efforts put in at either end.
| badge_id = name.lower().replace(" ", "-") | ||
| bad = ['"', "'", "(", ")", "*", "&", "?"] | ||
| replacements = dict(zip(bad, [""] * len(bad))) | ||
| replacements = dict(zip(bad, [""] * len(bad), strict=False)) |
There was a problem hiding this comment.
How does the reformatting of a backend code help a feature request for the frontend functionality?
There was a problem hiding this comment.
We expect our pull requests to be atomic in nature. This essentially means that one may have more pull requests against one issue ticket, but one pull request must address a maximum of one issue ticket.
This PR implements the frontend UI for performing CRUD operations on the ISSUERS relation.
Key changes:
Added issuer creation form componentAdded issuer update form componentIntegrated API calls using RTK QueryUpdated Governor page to include issuer management sectionChanges:
New Components:
frontend/src/crud/issuers/create.jsxfrontend/src/crud/issuers/update.jsxAPI Integration:
creationIssuer mutationupdationIssuer mutationUI Integration:
Issuer sectioninfrontend/src/routesFixes #681