Contribute to Open Source. Search issue labels to find the right project for you!

Add support for using the same email address in multiple Zulip organizations

zulip/zulip

Zulip now supports putting different realms on different subdomains via the settings.REALMS_HAVE_SUBDOMAINS setting.

It would be awesome to now make it possible to actually reuse the same email address with multiple accounts.

This will likely be a huge refactoring. Fortunately, we already encapsulate basically every email-related lookup using get_user_profile_by_email, so we just need to replace them with something that accepts a realm argument; e.g. get_user(realm, email). There are about 450 references in tests and 125 not in tests. For the tests part, I think working on this would be a good time to create a better extraction for getting the “hamlet@zulip.com” user, for example.

(zulip-py3-venv) tabbott@zaset:~/zulip$ git grep get_user_profile_by_email | wc
    576    2303   54907
(zulip-py3-venv) tabbott@zaset:~/zulip$ git grep get_user_profile_by_email | grep test | wc
    450    1769   42665

May be a good PyCon project :)

Updated 24/03/2017 06:43 2 Comments

compose: Change compose box to use a handlebars template.

zulip/zulip

Currently, the message edit box and the compose box have completely different styling. We’d like to make the message edit box look more like the compose box. The first step for this is to render the compose box via a handlebars template, so that we can then reuse that template for the message editing box.

Much of the relevant compose box code is in static/js/compose.js.

Updated 23/03/2017 02:24 4 Comments

Break some JS circular dependencies.

zulip/zulip

Talk to @showell or @timabbott before working on this ticket.

This is a list of dependencies we can try to break in our JS code.

ping @tommyip

~~~ activity.js activity.process_loaded_messages message_fetch.js

compose.js compose.stream_name composebox_typeahead.js

compose.send_message_success
    echo.js

compose.finish
    composebox_typeahead.js

compose.transmit_message
    echo.js

compose.mark_rendered_content_disparity
    echo.js

compose.empty_topic_placeholder
    message_edit.js
    message_store.js

compose.snapshot_message
    drafts.js

compose.autosize_textarea
    drafts.js

compose.report_as_received
    echo.js
    message_util.js

echo.js echo.apply_markdown drafts.js

hashchange.js hashchange.exit_modal subs.js

hashchange.save_narrow
    narrow.js

hashchange.operators_to_hash
    narrow.js

message_edit.js message_edit.get_editability message_list.js

message_events.js message_events.insert_new_messages echo.js

modals.js modals.close_modal drafts.js

narrow.js narrow.topic topic_list.js

narrow.activate
    topic_list.js

narrow.show_empty_narrow_message
    message_fetch.js

narrow.stream
    navigate.js

narrow.huddle_with_uri
    activity.js

narrow.narrowed_to_pms
    typing.js

narrow.operators
    typing.js

narrow.by_stream_uri
    stream_data.js

narrow.filter
    pm_list.js

narrow.by_subject
    notifications.js

narrow.public_operators
    message_fetch.js

narrow.active
    echo.js
    message_fetch.js
    pm_list.js
    unread.js

narrow.by_stream_subject_uri
    topic_list.js

narrow.pm_with_uri
    activity.js
    pm_list.js

narrow.by
    activity.js
    navigate.js

navigate.js navigate.scroll_to_selected resize.js

notifications.js notifications.update_pm_count unread_ui.js

notifications.close_notification
    unread_ops.js

notifications.update_title_count
    unread_ui.js

notifications.redraw_title
    narrow.js

notifications.window_has_focus
    unread_ops.js

notifications.clear_compose_notifications
    compose.js
    narrow.js

pm_list.js pm_list.update_private_messages echo.js message_fetch.js

popovers.js popovers.set_userlist_placement resize.js

popovers.hide_all
    echo.js
    pm_list.js
    resize.js
    stream_list.js

popovers.hide_reactions_popover
    reactions.js

reload.js reload.initiate channel.js compose.js server_events.js

reload.is_in_progress
    compose.js
    server_events.js

reload.is_pending
    compose.js

search.js search.update_button_visibility narrow.js

server_events.js server_events.home_view_loaded message_fetch.js

server_events.force_get_events
    tutorial.js

server_events.assert_get_events_running
    compose.js

server_events.restart_get_events
    compose.js
    socket.js

stream_events.js stream_events.mark_unsubscribed subs.js

stream_events.mark_subscribed
    subs.js

stream_list.js stream_list.get_stream_li navigate.js

stream_list.update_dom_with_unread_counts
    unread_ui.js

stream_list.update_streams_sidebar
    message_fetch.js

subs.js subs.set_color stream_color.js

subs.check_stream_existence
    compose.js

subs.invite_user_to_stream
    compose.js

tab_bar.js tab_bar.colorize_tab_bar stream_color.js

tutorial.js tutorial.defer message_fetch.js

tutorial.is_running
    message_fetch.js

ui.js ui.hide_loading_more_messages_indicator message_fetch.js narrow.js

ui.show_local_message_arrived
    echo.js

ui.show_message_failed
    echo.js

ui.show_loading_more_messages_indicator
    message_fetch.js
    narrow.js

ui.actively_scrolling
    narrow.js

ui.replace_emoji_with_text
    notifications.js

ui.show_failed_message_success
    echo.js

unread_ops.js unread_ops.process_visible notifications.js

Updated 24/03/2017 07:06 3 Comments

Various issues uncovered by fix that caused additional execution of test code

zulip/zulip

That fix was pull request #4162.
Issues remain that would cause failure of various race tests, not on a race condition but due to other issues. One is that comparison of states would fail due to different values of avatar_url being returned for each call to fetch_initial_state_data. The avatar_url fields were deleted from the states to allow these tests to pass. 3 other tests are now commented out because otherwise tests would fail, at least in part related to issues with the realm_bots state field.

Updated 23/03/2017 15:49 2 Comments

refactor/test: Clean up hotkey.js.

zulip/zulip

We can accomplish a couple goals with static/js/hotkey.js:

  • Eliminate nearly all jQuery calls (besides keydown/keypress) by delegating to other modules like settings.js, compose.js, popover.js, etc.
  • Add more coverage to hotkey.js (via frontend_tests/node_tests/hotkey.js)

This is a good task for a relative newbie who has a little bit of experience with Zulip node tests.

The best way to accomplish this is with a series of pretty small commits. You will want to be thorough with both adding automated tests and manually testing your changes.

Updated 27/03/2017 05:24 3 Comments

Reduce duplicated code between compose and message edit.

zulip/zulip

This is a big but important project to refactor most of the functionality out of the compose pathway into generic functions that can then be called by both the compose and message edit pathway. Eventually, we may even want to use the same handlebars template and most of the same CSS.

This is a good project for someone who has already done several smaller Zulip projects, and is comfortable with git rebase -i and regression testing.

Updated 01/03/2017 22:53 3 Comments

Optimize the number of steps involved in adding a new organization setting

zulip/zulip

We now have a ton of organization settings, and there’s always interest in adding more. In http://zulip.readthedocs.io/en/latest/new-feature-tutorial.html, we have a clear tutorial for how to add such a feature, but it involves a lot of semi-duplicated code. It’d be great to support some sort of declarative mechanism where we could declare that e.g. add_emoji_by_admins_only is a checkbox/boolean field, and have that imply the logic in:

  • [x] do_set_realm_add_emoji_by_admins_only (zerver/lib/actions.py)
  • [ ] The conditional block at least, in zerver/views/realm.py (though the latter will involve doing something clever, I think).
  • [ ] The basic API endpoint tests in test_realm.py
  • [ ] A test in test_update_realm_api in test_events.py.
  • [ ] Maybe even having an meta-handlebars template that can generate the input-group element for the checkbox, where one just passes in the field name, title and label, and that takes care of the rest.

https://github.com/zulip/zulip/pull/3833/files is a good example of what a current new realm setting PR looks like. With these changes, we could probably eliminate 2/3 of the code added for each new setting; with a bit more work I bet we could get it pretty close to 0.

In terms of how to do the declarations, I’d start with trying to just have an attribute on the Realm class; we might end up making a control class like CountStat in the future.

Updated 24/03/2017 01:05 1 Comments

Simplify the page_params generation logic

zulip/zulip

In zerver/views/home.py, we construct this giant data structure, page_params, which is based on the data returned from the POST /register API response, but also adds a bunch of other random things.

Ideally, I’d like to move to world where there’s at most a few items (e.g. web-only settings) that are accessible there which are not accessible from the Zulip API /register endpoint.

I did a bit of work towards this goal just now, organizing the data in page_params by where it comes from. There’s a few things that I think we should do to make this code path a lot simpler: * [ ] Move the realm data section down into fetch_initial_state_data so it entirely pulls from register_ret, and then move those items into page_params_core_fields. * [ ] Move the user_profile data section down into fetch_initial_state_data so it entirely pulls from register_ret, and then move those items into page_params_core_fields. This may entail some renaming of fields on the frontend to match the backend. * [ ] Rename the JS on the frontend (and thus also page_params) for subbed_info, people_list, etc. to use the names matching what is returned by the Zulip API, and then move those items into page_params_core_fields.

Once those are finished, the manual code in zerver/views/home.py will be significantly decreased, and we can think about further steps in a follow-up issue.

This is a sensitive refactoring, so it should be done in small, fine-grained commits, like e86ed89986ee1eec81f307779b7913c4ef3e6cf1 that just adjust one setting and pass tests (HomeTest will need to be updated in most such commits). But I think it won’t be a huge project if done carefully.

This project will make our life easier for a future project to cut down on the number of manual steps involved in a new realm setting in the new feature tutorial (see https://github.com/zulip/zulip/issues/3854).

Updated 01/03/2017 05:59

Clean up use of get_active_user_dicts_for_realm

zulip/zulip

get_active_user_dicts_for_realm is used in a number of places where we really don’t need something that expensive:
* Change the active_user_ids function to use its own independent cache. The list of user IDs is a small fraction of the total data, so we’d save a lot of wasted processing by doing this. * Complete #374. * Eliminate the call in add_subscriptions_backend. We don’t really need that old notification code. We can clearly remove it after making realm.notifications_stream something that can be changed by a realm administrator via the realm UI. I’ve opened #3708 to track this.

This is a continuation of #375 part (4), which I’m closing now.

Updated 17/02/2017 07:39

performance: Un-denormalize back end message caches.

zulip/zulip

We fetch several fields related to the “sender” when we call Message.get_raw_db_rows and we put them into our caches, so our user caches and messages caches are effectively denormalized, and thus bulkier and harder to invalidate when user info changes. We can probably slim the message queries/caches and hydrate user fields as needed from the user caches using bulk fetches from memcached. We may take a small speed hit on this, but we would avoid having stale data in the message cache (to the extent that the user cache is up to date, of course).

We can also think about slimming the data we send over the wire itself, by, for example, giving the browser client an option to not get the user fields (besides id) and let the browser do its own message hydration. We would probably continue to send “wide” data to other API clients.

https://chat.zulip.org/#narrow/near/143542/stream/backend/topic/Message.2Eget_raw_db_rows

Updated 02/02/2017 20:24

Certbot library usage

certbot/certbot

Would it be possible to refactor certbot to be more of a library+tool than just a tool? Or two packages python-certbot and certbot, with the latter having the current functionality but using the former at its core?

Referencing #1793, #358, #1299 as semi-related items, although they are more focused on refactoring the certbot the tool. I’m interested in a refactor into a libary+tool in one package, or separate packages for each.

I think this would lead to greater uptake from hosting providers (easier implementation), as it becomes much simpler to integrate into a fully automated certificate generation process.

I created a wrapper around certbot[1], but I’d rather see a native Python implementation than running certbot in a separate process within Python as I’ve done. A short conversation on the topic here [2].

[1] https://github.com/jaddison/certbot_py [2] https://community.letsencrypt.org/t/python-wrapper-around-certbot-auto-0-10-0/25582/2

Updated 23/01/2017 19:25 6 Comments

REST API: sending private message should require recipients in list (JSON)

zulip/zulip

From https://chat.zulip.org/#narrow/stream/bots/subject/new.20private.20message.20via.20REST/near/110659 :

Andrea Longo: I’m testing sending private messages via REST (with Swagger) and I’m going by what api/endpoints/ says about functionality.

For to it says “In the case of a private message, a JSON-encoded list containing the usernames of the recipients.” But I can use either cordelia@zulip.com, othello@zulip.com or [ "cordelia@zulip.com", "othello@zulip.com" ] and both work.

Is this documentation out of date?

Tim Abbott: No, it originally worked the other way and we haven’t removed the backwards-compatibility code. We probably should.

Updated 02/02/2017 20:00 1 Comments

Refactor Zulip's email codepaths

zulip/zulip

Zulip currently sends the following emails. (path) means the email template is in (a subdirectory of) templates/path/. * invitation (confirmation) * prereg confirmation, i.e. at realm creation or registration (confirmation) * zephyr invitation (confirmation) * zephyr setup done (confirmation) * password reset (registration) * digest (zerver/emails) * followup day1 and day2 (zerver/emails) * invitation reminder (zerver/emails) * find team (zerver/emails) * missed message (zerver)

They seem to have been written at different points in Zulip’s development; some use the Django 1.6 style of sending email (we’re currently on Django 1.10), some have filenames ending in _html.txt thus avoiding our linter, some seem to go through similar parallel codepaths (e.g. functions called things like send_this_type_of_email), some only have text versions and not html versions, some have our logo and some don’t, some have support offers and some don’t, etc.

First priority is probably to ensure some homogeneity in the emails that are sent, by e.g. having a common header and/or footer if appropriate, always including the logo in the html versions, always having an html version, always having a verbose_support_offers clause, etc. Issue #3134 covers some of this. Something to keep in the back of the mind might be future translatability; currently none of our emails are translated.

Second priority (but much easier, so maybe do this first) is to set a good file naming convention (foo_email.{html,txt,subject} seems like a good one), and to ensure we’re using send_mail everywhere instead of the Django 1.6 style of sending email (covered in Issue #3132).

Probably third priority (only because it is the hardest) is to write common email code that everyone can use, or figure out some way (developer documentation, if nothing else) to make it easy for someone adding a new email to our system to know what to do. There are unfortunately at least two legitimately different types of email to send: queued email like digest, followup, invitation reminder, and maybe missed message, and then everything else.

Many parts of this could be done by anyone (including someone with little Zulip experience), so don’t feel like you need to tackle all of this to tackle some of it!

Updated 14/03/2017 22:01 7 Comments

Remove realm_str from session object for realm sign-up.

zulip/zulip

We have the following logic in zerver.views.__init__.py: def accounts_home_with_realm_str(request, realm_str): # type: (HttpRequest, str) -> HttpResponse if not settings.REALMS_HAVE_SUBDOMAINS and completely_open(get_realm_by_string_id(realm_str)): # You can sign up for a completely open realm through a # special registration path that contains the domain in the # URL. We store this information in the session rather than # elsewhere because we don't have control over URL or form # data for folks registering through OpenID. request.session["realm_str"] = realm_str return accounts_home(request) else: return HttpResponseRedirect(reverse('zerver.views.accounts_home'))

It would be great to no longer have to store the realm_str in the session, since it increases the security impact of a SESSION_KEY leak. I imagine the way to do this will be to pass the realm (or realm_str) through the google auth / github auth paths in zerver/views/auth.py, either as a parameter or by maintaining the URL. Most of the work for maintaining the URL has already been done as a part of the subdomains project over the summer.

This task would be better for a relatively experienced programmer, since registration/authentication bugs can be a bit subtle.

Updated 24/03/2017 07:05 1 Comments

Make our various CSS subsystem files obviously constrained to that component

zulip/zulip

We should make sure all of our supplementary CSS files (e.g. overlay.css) are setup and linted to only have a few top-level CSS rules (to avoid accidentally modifying other parts of the app).

I’m not sure exactly what the best mechanism would be, but a simple design would be to have a fixed structure of this form tracking the list of valid rules overlay.css: ['#overlay', ...] and then verify that every CSS rule is contained within one of the patterns defined there.

Updated 04/01/2017 01:25

Improve the error message in case of Login failure

zulip/zulip

If the email or password entered by the user is not correct the following message is sent from the server.

Your username or password is incorrect.

In case, the user has not entered an email address (no @ in the username field) we could give a more user friendly message. In future with LDAP support, users might be allowed to just enter a username(without domain). So, we could simply check for 2 cases in case of login failure: 1) there’s no @ in the email and 2) that LDAP feature is not enabled.

and send an appropriate error message.

Updated 24/03/2017 07:08 3 Comments

Refactor IDisplay interface

certbot/certbot

As of writing this, no third party plugins are using it. It’s getting a little gross with tons of arguments being passed in, many of which aren’t relevant anymore with the removal of dialog and lots of *args and **kwargs due to differences in FileDisplay and NoninteractiveDisplay. We should clean all this up while we have the chance.

Updated 09/03/2017 19:36

PEP-8 style errors in package/library imports in files.

zulip/zulip

There are files where the code doesn’t look good because violating PEP-8 style errors. Imports should be grouped in the following order:

  1. Standard library imports
  2. Related third party imports
  3. Local application/library specific imports

You should put a blank line between each group of imports.

Source: https://www.python.org/dev/peps/pep-0008/

Following the instructions would improve readability of the code IMO. Maybe https://github.com/PyCQA/pycodestyle can be used.

Updated 02/02/2017 19:55 5 Comments

Add ability to hide user email addresses

zulip/zulip

Sometimes, it’s desirable to hide user’s email addresses (for example, open source projects with GitHub OAuth login where contributors do not want to share the e-mail address).

The only UI I’ve found where addresses are shown in the frontend are private messages. How complicated would it be to remove all such occurences?

Updated 24/03/2017 07:02 1 Comments

Stop using print

certbot/certbot

In a few places in our code, we use print to bypass the line wrapping that IDisplay does. The problem with this is it requires us to duplicate code for things like -q/--quiet, making maintenance harder and increasing the chance for bugs.

Instead, we should either add an optional flag to the notification method to disable line wrapping or add a new method to IDisplay that doesn’t format output.

Updated 19/03/2017 02:30 7 Comments

Upgrade javascript dependencies and remove from being vendored in static/third/

zulip/zulip

Most of the javascript modules we have vendored under static/third could be fetched via npm with pinned versions just as well (we haven’t patched most of them). We should systematically replace as many of these as we can with entries in package.json (and upgrade them to current versions).

When doing so, we should be careful to use git log --follow on the relevant file to make sure Zulip doesn’t have any patches not present in upstream (any for any where we do, we should submit them upstream so we can stop maintaining forks of javascript modules!).

Updated 08/01/2017 03:44 2 Comments

Remove up to 49 obsolete CSS classes

zulip/zulip

The following CSS classes from zulip.css are probably, but not definitely, obsolete:

.actions_hovered
.actions_link
.bookend_tr
.edit-profile
.edit_content
.hidden-filter
.icon-vector-heart
.label_for_text
.loading_indicator_spinner
.loading_indicator_text
.logout
.message-edit-tooltip-inner
.message-pane
.message-right
.message_data
.message_inline_image
.messagebox-bottom
.messagebox-bottom-colorblock
.messages-collapse
.messages-expand
.open
.pointer
.position-relative
.private_message
.ps-scrollbar-y
.ps-scrollbar-y-rail
.root
.sender_name-in-status
.settings_committed
.sidebar-nav
.skinny-user-gravatar
.sp-container
.sp-palette-container
.sp-preview
.sp-replacer
.sticky
.stream
.streamname
.summary_colorblock
.summary_row
.summary_row_private_message
.tiny
.tooltip
.user-with-mobile
.user_idle
.user_offline

We should try to remove these from our CSS. For each one the process is something like this: - grep the code to make sure it’s not referenced - if it’s a compound thing like message-collapse, see if you can find code that builds it up - do git blame on the zulip.css file to get some context on when the class was introduced

If you are 95% confident the class is really obsolete, submit a PR with just removing the one class, so it’s easy to revert.

If the class is valid, but the code that refers to it is hard to track with grep, see if there is an easy fix to the code to make it more obvious that the class is used by either the JS code or our templates.

Otherwise, update the ticket with any info you came up with in terms of tracking down the class.

Updated 23/02/2017 08:04 9 Comments

Fix --csr by separating it from --no-lineages

certbot/certbot

Currently, the --csr flag breaks under many circumstances because it implies a whole bunch of variations on how certs are stored. None of that seems logically necessary. --csr can work perfectly well with storage.py (it should probably imply --domains <contents of CSR>, etc) and we can have another flag (perhaps --no-lineages or --dump-cert or something) that causes the store-certs-in-the-current-directory-without-lineages behaviour.

Updated 14/03/2017 01:05 6 Comments

Too many dependencies

certbot/certbot

Users/developers have reported that our project has too many Python dependencies. This is related to things being overly complex.

Here is pde’s attempt to characterize what our dependencies currently are:

  1. Purely for certbot-auto and the things it needs to build, not for the python code itself (these no longer apply on recent OSes where we have packages):

    git gcc python-dev libffi-dev libssl-dev setuptools virtualenv

  2. Essential OS packages python openssl
  3. Semi-essential OS packages: dialog
  4. Needed for specific plugins:

    python-psutil (standalone – see what has bound :80 or :443) (now only “recommended”) libaugeas0 (apache – parse & edit conf files) python-augeas

  5. Python dependencies:

    ConfigArgParse (for reading config files) configobj (for writing & reading renewal conf files) python-cryptography (>= 0.8) PyOpenSSL (>= 0.15) (>= 0.13) pythondialog ndg-httpsclient (for working around TLS insecurities in older Python versions) requests six

  6. Python dependencies specifically related to time processing. It seems absurd that there are four of these:

    werkzeug pytz parsedatetime pyicu (needed by parsedatetime) pyrfc3339

  7. Python dependencies for python-cryptography: idna enum34 ipaddress cffi pycparser (build dep only, but affects letsencrypt-auto) pyasn1
  8. These were used to define plugin interfaces. Not sure if that was wise, but it’s probably annoying to third parties if we ditch them:

    zope.components zope.interface zope.event

  9. Development and doc-building dependencies. For reasons I don’t yet understand, pip always fetches some of these too:

    mock funcsigs (mock depends on this) pbr (mock depends on this) sphinxdocs

Updated 26/02/2017 10:31 12 Comments

Fork me on GitHub