-
Notifications
You must be signed in to change notification settings - Fork 12
Add dummy data for development #5
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
Conversation
Previously we'd include the ID of the commitfest in the URL of the patch. In 9f12a5e we introduced a stable URL for patches that would redirect to the one for the latest commitfest. This starts to use that URL as the valid only URL for a patch (with the previous URL redirecting to this one). The reasoning behind this is that the old approach resulted in N different URLs for each patch, which all showed the exact same patch information. The only difference between all these URLs would be the breadcrumb at the top of the page. The only benefit of that approach is that if you're on an old commitfest, and click a link there, then the breadcrumb will bring you back to where you came from. Since people rarely have a reason to browse closed commitfests, the that benefit seems pretty small. Especially because people can just as well press their browser back button, in that case. The problems that these N links cause seem much more impactful to most users: 1. If you click an old link to a cf entry (e.g. one from an email in the archives), then the breadcrumb will contain some arbitrarily old commitfest. It seems much more useful to have the breadcrumb show the commitfest that the patch is currently active in (or got committed/rejected in). 2. If you arrive on such an old link you also won't be able to change the status. Instead you'd get a message like: "The status of this patch cannot be changed in this commitfest. You must modify it in the one where it's open!". Which requires you to go to the latest page. 3. Places that use the stable URLs require an extra round-trip to actually get to the patch page. 4. It's a bit confusing that old pages of a patch still get updated with all the new information, i.e. why have all these pages if they contain the exact same content. 5. Problem 3 is generally also bad for Search Engine Optimization (SEO), for now we don't care much about that though. Finally this also changes the links on the patch page itself for each of the commitfests that a patch has been part of. Those links were already rather useless, since all they effectively did was change the breadcrumb. But with this new commit, they wouldn't even do that anymore, and simply redirect to the current page. So now they start pointing to the commitfest itself, which seems more useful behaviour anyway.
The bottom dropdowns on the patch page would expand downwards, requiring the user to scroll down to see and click any of the buttons in the dropdown. With this change these are changed into "dropup" menus, so the expand upwards.
Thank you for working on this. How did you generate this mock data? I'm assuming you have created some script? I think that script should be part of the PR, because we'll likely want to improve/update it in the future (e.g. when adding new tables/columns) |
@JelteF I updated the PR description and the README with more information! This is now a working POC, ie I can start from a fresh install and seed a working database now with these files. |
Thanks for the clear readme. How did you generate the archive_data.json file? |
I took some data from https://commitfest.postgresql.org/ajax/getThreads/?s=&a=1 and then made some manual modifications to anonymize it. |
I made a few changes, let me know what you think. |
To be clear it worked great! The main things I changed were:
|
Also it seems like you haven't installed https://editorconfig.org/, because there were some trailing whitespaces in the README. |
Wow, awesome! I’m especially impressed that you figured out the recursion
error. This all looks much better.
|
I updated the README and also verified that the dummy data update works in a clean install, thank you! |
This PR implements a way to use dummy data for development in two ways: 1. Dump a working dev database with Django's `dumpdata` command. We dump the `auth` and `commitfest` modules separately. This data can likewise be reloaded when starting from scratch with the corresponding `loaddata` commands (see the README.) 2. Mocks the archives server, to allow users to search and add sample mailing threads to their patches. To avoid an infinite recursion error this change also required moving the ManyToMany relationship between MailThread and Patch from the MailThread to Patch side. --------- Co-authored-by: Jelte Fennema-Nio <[email protected]>
This PR implements a way to use dummy data for development in two ways:
dumpdata
command. We dump theauth
andcommitfest
modules separately. This data can likewise be reloaded when starting from scratch with the correspondingloaddata
commands (see the README.)There was an additional problem with this: The dumpdata django command would throw an infinite recursion error with our database schema. To solve this error this change also moves the ManyToMany relationship between MailThread and Patch from the MailThread side to Patch side.