Skip to content

Return count when row_count is selected #97

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Return count when row_count is selected #97

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/Command/Api4Command.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ protected function execute(InputInterface $input, OutputInterface $output) {
throw new \RuntimeException("Please enable APIv4 before running APIv4 commands.");
}

list($entity, $action) = explode('.', $input->getArgument('Entity.action'));
[$entity, $action] = explode('.', $input->getArgument('Entity.action'));
$params = $this->parseParams($input);
if ($output->getVerbosity() >= OutputInterface::VERBOSITY_VERBOSE || $input->getOption('dry-run')) {
$output->writeln("{$I}Entity{$_I}: {$C}$entity{$_C}");
Expand All @@ -149,11 +149,14 @@ protected function execute(InputInterface $input, OutputInterface $output) {
$columns = empty($params['select']) ? array_keys($result->first()) : $params['select'];
$this->sendTable($input, $output, (array) $result, $columns);
}
elseif (!empty($params['select']) && $params['select'] === ['row_count']) {
$this->sendResult($input, $output, $result->count());
}
Copy link
Member

Choose a reason for hiding this comment

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

Aah, interesting, so you can select the row_count even though it's not actually a field in the result. This feels weird.... but OK, it appears to work w/AJAX. The AJAX-bindings wrap all results in an extra layer of JSON ({values: ..., count: ...}).

Of course, the patch above works if you select only the row_count. If you want to get data as well as row_count, then one needs to be more like the AJAX-bindings. I guess one could port the AJAX technique -- and for compatibility purposes, treat it as a flag, e.g.

cv api4 Contact.get +s display_name,row_count --wrap
{
  "count": 123,
  "values": [
    {first},
    {second}
  ]
}

or like:

cv api4 Contact.get +s row_count --header
{"count": 123}
[
  {first},
  {second}
]

FWIW, even with the wrapping, there's still a problem with chaining. You can see this in AJAX calls:

Screen Shot 2021-05-14 at 6 07 57 PM

I guess a couple questions:

  • Looking forward -- Are there more header/wrapper fields that we should expect?
  • Looking forward -- Do you expect some kind change/fix for the chaining issue?

If the answer is "No" on both fronts, then I'd be OK with this kind of special case. Otherwise, it's probably better to make the serialization formats more aligned.

else {
$this->sendResult($input, $output, $result);
}

return empty($result['is_error']) ? 0 : 1;
return 0;
Copy link
Member

Choose a reason for hiding this comment

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

I don't see how this part is relevant. Note that the result-code is important for handling errors in bash scripts. Returning 0 signals normal/successful exit. Errors should return non-zero. (Some tools go all-out and define various numeric-error-codes for different conditions. That's a bit of PITA here, which is why it merely returns 1.)

Some examples:

set -e
cd path/to/my/ext
git pull
cv upgrade:db
cd ..
set -e
cv api4 Contact.get +s display_name --out=list | while read NAME; do
   echo "Say hello to $NAME for me" 
done
cv flush && cv ext:list -R

In the first two cases, set -e means "If any command fails, stop execution and report the problem". If the third case, the && means "If left-command succeeds, then run right-command." They all rely on the exit-code.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's just cleanup. This function looks to have been copy/pasted from the api3 function. Api3 returns an is_error key. Apiv4 does not.

}

/**
Expand Down