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.

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

Jump to bottom

api: Sync updateStream and createStream with their API docs #5341

Merged
merged 16 commits into from
Apr 19, 2022
Merged

api: Sync updateStream and createStream with their API docs #5341

merged 16 commits into from
Apr 19, 2022

Commits on Apr 19, 2022

  1. api.updateStream [nfc]: Accept multiple properties at once

    @chrisbobbe
    chrisbobbe committed Apr 19, 2022
    Configuration menu
    Copy the full SHA
    65ddf3f View commit details
    Browse the repository at this point in the history

  2. streamsActions: Send one update-stream request for all changed proper… 

    …ties

Instead of up to three in sequence, one per changed property.
    @chrisbobbe
    chrisbobbe committed Apr 19, 2022
    Configuration menu
    Copy the full SHA
    be12766 View commit details
    Browse the repository at this point in the history

  3. api types [nfc]: Express updateStream's params by referring to Stream… 

    … type
    @chrisbobbe
    chrisbobbe committed Apr 19, 2022
    Configuration menu
    Copy the full SHA
    3fc7aa3 View commit details
    Browse the repository at this point in the history

  4. modelTypes [nfc]: Order Stream type's properties to follow a doc

    @chrisbobbe
    chrisbobbe committed Apr 19, 2022
    Configuration menu
    Copy the full SHA
    eb50271 View commit details
    Browse the repository at this point in the history

  5. modelTypes: Update Stream type, to match streams initial data at FL… 

    … 121
    @chrisbobbe
    chrisbobbe committed Apr 19, 2022
    Configuration menu
    Copy the full SHA
    dd6fea1 View commit details
    Browse the repository at this point in the history

  6. api types: Sync updateStream's params with API doc at FL 121

    @chrisbobbe
    chrisbobbe committed Apr 19, 2022
    Configuration menu
    Copy the full SHA
    0c56883 View commit details
    Browse the repository at this point in the history

  7. stream [nfc]: Assimilate to name "invite_only" in create/edit stream … 

    …logic

We'll be adding more stream properties here soon, and it'll be best
to avoid multiplying the number of different names.
    @gnprice @chrisbobbe
    gnprice authored and chrisbobbe committed Apr 19, 2022
    Configuration menu
    Copy the full SHA
    2668bde View commit details
    Browse the repository at this point in the history

  8. api [nfc]: Have createStream use params objects 

    One for stream attributes, and one for miscellaneous options.

The create-stream API is a little awkward in that it piggy-backs on
the subscribe-to-stream API. Here's the doc:
  https://zulip.com/api/create-stream

> You can create a stream using Zulip's REST API by submitting a
> [subscribe](/api/subscribe) request with a stream name that
> doesn't yet exist and passing appropriate parameters to define the
> initial configuration of the new stream.

So, make our code a bit clearer by bundling the attributes of the
to-be-created stream into an object. We're about to add more
attributes, to sync with the API doc.

While we're at it, order the attributes by their appearance in the
doc.
    @chrisbobbe
    chrisbobbe committed Apr 19, 2022
    Configuration menu
    Copy the full SHA
    08c0697 View commit details
    Browse the repository at this point in the history

  9. api [nfc]: Remove some param-defaulting within our api.createStream b… 

    …inding

I think the only good reason to have such fallback code at this
layer would be that we don't trust the server to use reasonable
fallbacks itself.

There are certainly awkward things about this endpoint -- it's doing
double duty as "subscribe-to-stream" and "create-a-stream" -- but I
see no reason to distrust the server's defaults for these params,
which set attributes on a created stream. Its choices currently
match ours, but they could reasonably change, and we'd be fine
deferring to the server if they did.

NFC because, currently, this binding's only caller also applies the
same fallbacks. That might be a bad choice, but at least now that
choice isn't forced by the code that's designed to interface
directly with the server.
    @chrisbobbe
    chrisbobbe committed Apr 19, 2022
    Configuration menu
    Copy the full SHA
    94d7a59 View commit details
    Browse the repository at this point in the history

  10. api: Remove more param-defaulting within our api.createStream binding 

    With similar reasoning as in the previous commit, except these are
in `options` instead of `streamAttributes`.

This time, one of the properties doesn't have a default applied by
the single caller: `announce`. So this is a behavior change: we're
now sometimes omitting `announce`, which lets the server decide what
behavior to fall back on. Currently, it's documented as falling back
to `false` (and that should apply for all non-ancient servers):
  https://zulip.com/api/subscribe#parameter-announce
    @chrisbobbe
    chrisbobbe committed Apr 19, 2022
    Configuration menu
    Copy the full SHA
    9148ab3 View commit details
    Browse the repository at this point in the history

  11. CreateStreamScreen: Let server apply its own fallback for principals 

    This seems like a fine place to let the server control the behavior,
and it's documented to give the same behavior anyway. See
  https://zulip.com/api/subscribe#parameter-announce :

> […] If not provided, then the requesting user/bot is subscribed.

We can start passing `principals` again when we eventually have a UI
for choosing multiple users to be subscribed, as the web app does.

For now, it's redundant and unnecessary for us to request the same
behavior the server would give anyway. Especially since we're asking
for it in a way that's discouraged: we use an email instead of a
user ID.
    @chrisbobbe
    chrisbobbe committed Apr 19, 2022
    Configuration menu
    Copy the full SHA
    ffc44bc View commit details
    Browse the repository at this point in the history

  12. userSelectors [nfc]: Bring a comment up-to-date 

    Grepping shows that we actually use this in several places now.
    @chrisbobbe
    chrisbobbe committed Apr 19, 2022
    Configuration menu
    Copy the full SHA
    2a5bcd2 View commit details
    Browse the repository at this point in the history

  13. userSelectors [nfc]: Remove now-unused getOwnEmail 

    Related: zulip#3764
    @chrisbobbe
    chrisbobbe committed Apr 19, 2022
    Configuration menu
    Copy the full SHA
    aba93ee View commit details
    Browse the repository at this point in the history

  14. api types [nfc]: Express createStream's params by referring to Stream… 

    … type
    @chrisbobbe
    chrisbobbe committed Apr 19, 2022
    Configuration menu
    Copy the full SHA
    7fee95b View commit details
    Browse the repository at this point in the history

  15. api [nfc]: Use spreads, to reduce duplication

    @chrisbobbe
    chrisbobbe committed Apr 19, 2022
    Configuration menu
    Copy the full SHA
    d002af0 View commit details
    Browse the repository at this point in the history

  16. api types: Sync createStream's params with API doc at FL 121

    @chrisbobbe
    chrisbobbe committed Apr 19, 2022
    Configuration menu
    Copy the full SHA
    6eff27a View commit details
    Browse the repository at this point in the history