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

Added api.get.displayed_email_data() #51

Merged
13 commits merged into from May 2, 2014
Merged

Added api.get.displayed_email_data() #51

13 commits merged into from May 2, 2014

Conversation

ghost
Copy link

@ghost ghost commented Apr 23, 2014

Sequel to #48.

2 functions related to conversation view:

  • api.get.displayed_email_data(): returns the whole thread data if convo is on (see below), or only one email data (the one displayed) if convo is off;
  • api.check.is_conversation_view(): returns true or false if convo is found to be respectively on or off. returns undefined otherwise;

2 utility functions (don't know about redundancy):

  • api.tools.extract_email_address(str): returns the first email address found in string;
  • api.tools.extract_name(str): returns the first name found in string;

With conversation view on

The whole thread is returned, but still there is filtering.

Deleted emails are removed from what's returned, because they aren't actually displayed.
If the thread is being read from trash, it's the opposite: only deleted emails are returned, because others won't be displayed here.

There is something missing: removing contacts from people_involved that are not in the displayed emails. That can be discussed. But I guess we want to know details only about displayed elements.

With conversation view off

api.get.displayed_email_data() takes care of filtering all the thread metadata, but only keeps the first email that is considered displayed. For that we look for CSS classes existing with email ids.

total_emails is fixed at 1, and both threads and total_threads contain a single element.

The subject property is the one of the thread ; no "Re: " prepended even if there's one on the current email.

The people_involved array is re-built with what is found in the email considered displayed: from, from_email and every element of to (from which we extract a name and an email address, to look like what api.get.email_data() returns).

@ghost ghost added the enhancement label Apr 26, 2014
@ghost ghost mentioned this pull request Apr 26, 2014
}
}
}
else { // Supposing only one displayed email.
Copy link
Owner

Choose a reason for hiding this comment

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

I dont think we need this else clause the helper tool functions at all. If the conversation view is turned off, you can return api.get.email_data() and it'll have the same response. Am I missing something?

@KartikTalwar
Copy link
Owner

Hey @g8g3 check out my comment about the else clause. Let me know if i understood it correctly. Other than that, this looks good to me! Feel free to merge it in yourself! :)

@ghost
Copy link
Author

ghost commented Apr 29, 2014

Hi,

I'm not entirely sure of which context you're in, but I'll try to summarize my intent.

First, we're OK that api.get.email_data() will always return all the emails data, even when we're with conversation view off and that there's only one email displayed.

Then,

  • in the if scope, we know conversation view is on. So, as api.get.displayed_email_data() suggests, we'll use api.get.email_data() (because the whole thread is taken into account) but also care about removing emails that can't actually be displayed: the deleted ones (or those not deleted, when in trash);
  • in the else scope, conversation view is off. So, based on api.get.displayed_email_data() purpose, we want to return the only email that is displayed. api.get.email_data() can't tell us what's "physically" displayed, thus we look for it through CSS. And that's it.

To me these are two separate cases. Sorry if I didn't get your point. (:
I'll wait before merging, to be sure.

Thanks for feedback.

@KartikTalwar
Copy link
Owner

@g8g3 I'm following this so far. What I meant to say was that if the conversation view is off, and you go on a single email and run api.get.email_data() or api.get.email_data(id_from_url_hash) you get the same response.

From my understanding, there is a thread_id and an email_id. With conversation view off, the url of an email is the email_id, so requesting data for that id only returns info of that email. That screenshot in my last comment is my inbox with conversation view off and those 2 commands. From my inspection, the responses are the same.

Could you verify that on your end as well and let me know if somethings different?

}
}

if (flag_value !== undefined) {
Copy link
Owner

Choose a reason for hiding this comment

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

Instead of this entire if check you can just return flag_value == "1". The true false order in var values is also opposite

Copy link
Author

Choose a reason for hiding this comment

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

Yes.
What I wanted initially, as we can't be 100% sure of which values correspond to which context, was to have a single object values with each possible flag value (sometimes it's 0/1, sometimes true/false, etc.).

At this point, returning using a simple explicit condition might indeed be simpler. I haven't noted other possible values.

Just: in my case it really is "0" when conversation view is on, and "1" when it's off.

@ghost
Copy link
Author

ghost commented Apr 30, 2014

What you say makes sense, only I couldn't get api.get.email_data(email_id) to work that way:

gmail console

With conversation view off, and in a thread containing 3 emails, asking for a specific email_id doesn't matter; I still get the whole thread. That's why I bothered filtering.

I don't know if it's broken in my case, or if your previous screenshot was taken with a single-email thread (in which case you necessarily have only that one retrieved).
I didn't look into how api.get.email_data() is built too much. But yeah, having it retrieve only one email is what's expected indeed.

@KartikTalwar
Copy link
Owner

@g8g3 looks like it is a different case so we'll need to keep explicit dom based filtering. Could you take care of the merge conflict and merge this into master yourself :)

ghost pushed a commit that referenced this pull request May 2, 2014
Added `api.get.displayed_email_data()`
@ghost ghost merged commit 23c93e7 into KartikTalwar:master May 2, 2014
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant