DT-458: Add ability for admins to export stories regardless of status#333
Merged
Conversation
torresga
marked this pull request as ready for review
October 23, 2024 14:42
JuanVqz
force-pushed
the
feature/DT-458-admin-export-stories
branch
from
July 21, 2026 00:02
a341873 to
2f8ca29
Compare
JuanVqz
added a commit
that referenced
this pull request
Jul 21, 2026
- Only render the Select All checkbox when the project is unlocked, matching Add a Story and the bulk-delete control (addresses the review question about the unlocked check). Add a spec that it is hidden on a locked project. - Make updateSelectAllStatus null-safe so toggling a row checkbox on a locked project (where Select All is absent) no longer throws. - Clean up the extracted updateBulkDeleteStatus indentation and file newline. - De-flake the madmin projects spec via CGI.escapeHTML (same fix as PR #333).
JuanVqz
force-pushed
the
feature/DT-458-admin-export-stories
branch
from
July 21, 2026 18:08
2f8ca29 to
94e5952
Compare
…her or not there are comments
…nd and removing the other if statement
- Only honor export_all when current_user is an admin, so the param can't be forged by a non-admin to export non-approved stories (the view-only guard was not enough). Add a non-admin guard spec and sign in as an admin in the admin spec, which previously never actually authenticated as one. - Simplify export: drop the 4-way branch and the unused export_all argument, and move generate_csv under private. - Fix a flaky madmin projects spec by comparing against the HTML-escaped title (Faker names with apostrophes were escaped in the page and failed the match).
- Eager-load comment authors (includes(comments: :user)) so generate_csv does not fire one query per comment for display_name. - Collapse all of a story's comments into a single quoted cell instead of spilling into a variable number of unlabeled trailing columns, so every row has a uniform width. Header renamed to "comments". - Add a spec for an admin exporting all stories with comments, covering that comments on non-approved stories are included.
JuanVqz
force-pushed
the
feature/DT-458-admin-export-stories
branch
from
July 21, 2026 19:27
94e5952 to
cb15a91
Compare
JuanVqz
approved these changes
Jul 21, 2026
Member
|
QA Passed, but it is missing the status column; otherwise, we won't know what has been rejected, approved or pending, so, adding the column now. 
|
Include each story's status in the export so an all-statuses export is readable (previously there was no way to tell approved from pending/rejected). Order the columns id, position, status, title, description, comments. Import matches columns by name, so the new column and ordering do not affect it.
Member
|
Now include the status, approved, and as soon as it's green, I'll merge it. 
|
JuanVqz
added a commit
that referenced
this pull request
Jul 21, 2026
- Only render the Select All checkbox when the project is unlocked, matching Add a Story and the bulk-delete control (addresses the review question about the unlocked check). Add a spec that it is hidden on a locked project. - Make updateSelectAllStatus null-safe so toggling a row checkbox on a locked project (where Select All is absent) no longer throws. - Clean up the extracted updateBulkDeleteStatus indentation and file newline. - De-flake the madmin projects spec via CGI.escapeHTML (same fix as PR #333).
JuanVqz
added a commit
that referenced
this pull request
Jul 21, 2026
* Adds Select All checkbox and functionality * Adds indeterminate state if not all checkboxes are selected * Adds specs to test Select All checkbox * Gate Select All on unlocked projects and harden select-all JS - Only render the Select All checkbox when the project is unlocked, matching Add a Story and the bulk-delete control (addresses the review question about the unlocked check). Add a spec that it is hidden on a locked project. - Make updateSelectAllStatus null-safe so toggling a row checkbox on a locked project (where Select All is absent) no longer throws. - Clean up the extracted updateBulkDeleteStatus indentation and file newline. - De-flake the madmin projects spec via CGI.escapeHTML (same fix as PR #333). * Assert Bulk Delete stays disabled on a locked project when a story is selected Documents that updateBulkDeleteStatus's unlocked guard is load-bearing: story checkboxes still render on locked projects, so without the guard, selecting a story would enable Bulk Delete. * Move Select All into a dedicated leftmost checkbox column Give the checkboxes their own column with a bold 'Select All' header, instead of tucking the control into the right-hand actions cell where it was hard to find. Add the sixth grid track for the new column and shift the story-title filter and estimate-position helper to account for it. --------- Co-authored-by: Juan Vásquez <juan@ombulabs.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Jira Ticket
https://ombulabs.atlassian.net/browse/DT-458
Motivation / Context
Currently, stories are only exported to CSV if they have an accepted status. This feature allows admin users to export stories regardless of their status. Non-admins should only be able to export approved stories.
I also refactored the
exportmethod because Code Climate was complaining.Additional changes from review
Follow-up improvements made while reviewing the PR:
export_allonly whencurrent_user.admin?, so a non-admin cannot forge?export_all=1to download non-approved stories. Added a spec for that forged-param case.user.admin = truewithout signing in as that user, so it never actually exercised admin behavior. It now signs in as an admin.export. Replaced the four-way conditional with two booleans (with_comments,export_all) and a single call togenerate_csv, and movedgenerate_csvtoprivate.includes(comments: :user)), instead of firing one query per comment fordisplay_name.commentscell (newline-separated) instead of spilling into a variable number of unlabeled trailing columns, so every row has the same width.statuscolumn. Since we can now export every status, the CSV includes each story's status so an all-statuses export is actually readable. Columns are orderedid, position, status, title, description, comments. Import matches columns by name, so the new column and ordering don't affect it.QA / Testing Instructions
As an admin:
commentscolumn, populated for non-approved stories as well.As a non-admin:
5. The Export all stories checkbox is not shown; export returns approved stories only.
6. Even hitting
…/stories/export?export_all=1directly returns approved stories only.Screenshots:
I will abide by the code of conduct.