-
-
Notifications
You must be signed in to change notification settings - Fork 27
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
1305 faq admin refactoring #1397
Conversation
…into 1305-faq-admin-take-4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I downloaded and ran your branch. It is working pretty well. Here are some things that need work:
- You are able to edit categories and FAQs on-screen and save the changes when you aren't even logged in, if you make changes to categories and/or FAQs and press the "Edit Section" button twice.
- When I ran the DB migration on my local database, the migration ending in "1230" failed on the first command with a SQL error that a Foreign key referenced the FaqCategory table, so it could not be deleted. I was able to fix it by swapping the first two SQL commands in that script - deleting the FaqCategory table before the Faq table, then running
npm run flyway:repair
to accept the modified migration script and thennpm run flyway:migrate
to finish the migration. - The database migration completely wipes out the existing Categories and FAQs, which took a lot of effort to create. We do have a migration script that could re-create the Faq and FaqCategory table data, except that you re-organized the table structure, so we need to map the FAQ table data into the FaqCategory.faqs JSON column, or create a different DM Migration script to re-populate the table with the initial FAQs.
- The "Edit Section" (not a good choice of wording - what is "Section" supposed to mean in this context?) should only be visible if you are an admin.
- Unless you are an admin and have pressed the "Edit Section" button, the Category, Question and Answer fields should all be read-only.
- The read-only rendering of the answers seems to render regular text the same as bold text - this seems to be because of the styling of the read-only answer - we probably need to remove the bold style applied to the read-only answer to fix this?
- The Expand All / Collapse All feature design has problems inherent in the design given to you. If only a few FAQs are expanded, you should have both the Expand All and Collapse All options. The way it is implemented now, you only get one of these options, and it (at least sometimes) does the opposite of what the label says.
…into 1305-faq-admin-take-4
Additional changes have been pushed to this PR that resolves @entrotech 's comments (#'s 1, 4, 5, & 7). I need to discuss how we want to handle the DB migration changes (#'s 2 & 3) going forward - will discuss with @entrotech on 7/26 meeting. This leaves #6 still open. I am working through this issue. Currently, the FAQ's answer is being saved as a string and rendered as a string; so the HTML modifications from the enriched text (such as bolded font, or hyperlinks, or other enrichment options) are not being rendered correctly. I will continue this work - hoping to have this issue #1305 fully completed by next week, at the latest the following week. cc: @Biuwa |
This Pull Request includes the following modifications:
<FaqView />
)insertAll
. This new procedure will delete the data stored infaqCategory
TABLE and replace it with theform state
onSave. With this work, I have addedfaqs
as acolumn
infaqCategory
TABLE. This eliminates the need to storefaqCategories
andfaqs
in separate tables (and eliminate the need to make requests for each and augment the FAQs into the categories state).