Skip to content
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

Surface missingCommissionedLengthReason field #476

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Fweddi
Copy link
Contributor

@Fweddi Fweddi commented Nov 1, 2024

We recently made commissioned length mandatory in Workflow and gave a way for users to opt out by clicking the Breaking News button.

This PR surfaces this choice in the Workflow UI (in the Commissioned Length column, renamed from the vague - if shorter - Commission).

To test

Run locally or deploy to CODE alongside https://github.com/guardian/workflow/pull/1123.

@Fweddi Fweddi force-pushed the fp/show-missing-commissioned-length-reason branch from 2060a3c to 5027e1e Compare November 1, 2024 16:49
@@ -311,6 +311,17 @@ const columnDefaults = [{
isNew: true,
isSortable: true,
sortField: ['statusInPrint']
},{
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-ordered so the commissionedLength can be next to the webWordCount (and so more easily comparable). But keeping webWordCount and printWordCount next to each other.

This is the inverse order of the Ophan screen (wordCount / commissionedLength), but hard to balance all the needs.

@Fweddi Fweddi marked this pull request as ready for review November 13, 2024 16:13
@Fweddi Fweddi requested a review from a team as a code owner November 13, 2024 16:13
Comment on lines 337 to 343
if ($scope.contentItem.missingCommissionedLengthReason !== null && $scope.contentItem.missingCommissionedLengthReason !== undefined) {
updateField("missingCommissionedLengthReason", null);
wfComposerService.deleteField(
$scope.contentItem.composerId,
"missingCommissionedLengthReason"
);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unset the missingCommissionedLengthReason when a commissioned length is set.

Comment on lines +335 to +336
// Don't allow commissioned length to be unset
if(newValue === "") return;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy for others to disagree with this choice. But there doesn't seem to be much benefit in allowing people to remove a commissioned length from a piece that has one set.

Theoretically it would allow users to correct a mistake, but in practice since we expect all (or almost all) pieces to have a commissioned length, it seems more likely that someone might mis-use this feature, i.e. by setting an arbitrary commissioned length on the Workflow form and then removing it to get round the mandate.

template: commissionedLengthTemplate,
active: true,
isSortable: true,
sortField: ['missingCommissionedLengthReason', 'commissionedLength']
Copy link
Contributor Author

@Fweddi Fweddi Nov 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you now order the Commissioned Length field by ascending, you should see missingCommissionedLengthReason first. And then very low Commissioned Length values. So will be easier to see mis-use of Commissioned Length.

Would be nice if we could order by descending to see very high Commissioned Length values, but this just shows empty values as highest. Perhaps some refactoring of the sort would help? (Should blank values be included or not in sort is the bigger question).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant