Need exporter improvements #122
Conversation
As the base_path's are dependent on the export order, and the default order is descending, which would publish the newest content first instead of last.
Fix off by one error, and add padding.
The data is numerical, so store it as a number.
This helps prevent the need exporter task timing out when running locally. It's probably ineffective in production, due to request duration limits elsewhere, but hopefully will not be necessary there.
This is consistent with Maslow, and looking at a few of the needs in the Need API, should provide better URLs also.
| end | ||
|
|
||
| def present_need_revision_group(need_revision_group, slug) | ||
| def present_need_revision_group(need_id, need_revision_group, slug) |
There was a problem hiding this comment.
Instead of doing this, could you get the need_id from the need_revision directly? In the console, NeedRevision.last.need_id gives me the need_id back.
There was a problem hiding this comment.
I did it this way because the need_id should be a constant over all of the revisions, and passing it in as a value makes the code read in the same way. I don't mind particularly though, I guess that getting it from the revisions is going to be fine in reality, and gets rid of an argument.
There was a problem hiding this comment.
ok, I can see why having it as an argument highlights it doesn't change across revisions, happy for you to keep it like this.
lib/needs_exporter.rb
Outdated
| @@ -41,6 +41,7 @@ def present_need_revision_group(need_revision_group, slug) | |||
|
|
|||
| { | |||
| title: goal, | |||
| description: "As a #{role}, I need to #{goal}, so that #{benefit}", | |||
There was a problem hiding this comment.
How about rebasing to delete commit 24a6bf9 instead?
There was a problem hiding this comment.
I've removed the two commits related to the description now.
There was a problem hiding this comment.
and, yeah, that makes more sense.
| p "#{index}/#{@needs.count}" | ||
| p "exported #{slug}" | ||
|
|
||
| padding = Math.log10(count).ceil |
There was a problem hiding this comment.
I don't understand what this padding does, could you explain in more detail?
There was a problem hiding this comment.
Its used in the string on the line below, maybe width would be a better name, as that is what its called in the format string documentation [1].
%#{padding}d will become something like %4d which will be substituted by format with the value given (index + 1), which will then become something like __14 (where the underscores are actually spaces), or 3000 with padding on the left of the string such that its 4 characters in width.
The desired width (or padding as its called above) is the number of characters required to display the largest possible index. The largest possible index is equal to the count, and to get the number of digits required to display it in base 10, you take the logarithm of that value in base 10, and round up.
All of this just gives you output formatted such that it moves around less, making it slightly easier to read.
1: https://ruby-doc.org/core-2.2.0/Kernel.html#method-i-format
Most of the information comes across in the publication state in the Publishing API.
The import endpoint has now changed to take a locale in the query parameters, and not the history entries.
As this makes for better titles.
So that it can be used in the title.
As this is more informative.
As this information can be stored in the Unpublishing explanation.
For gds-api-adapters, as this includes the code to talk to the import endpoint of the Publishing API.
cbaines
force-pushed
the
need-exporter-improvements
branch
from
099f112
to
f7527a5
Compare
As this better reflects what this function does.
This is better than using the status of the latest revision.
https://trello.com/c/jlXiAdmQ/7-add-needs-to-publishing-api