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

Relative Time now,weeks,months,years #4304

Merged
merged 5 commits into from
Nov 1, 2017

Conversation

montymxb
Copy link
Contributor

@montymxb montymxb commented Oct 30, 2017

Adds now as a recognized singular unit of time, indicating to use the current time as is.

I figure this would be handy in cases where you would like to fetch objects from a certain # of days/weeks ago, but not past the current time, or vice versa. On it's own it may be handy as well for getting anything that is still in the future.

Also adds an eslint-enable no-console mark where previously it was not closed.

@montymxb
Copy link
Contributor Author

Just while this is pending, is there any interest in adding in weeks and months as recognized units? I can add them in here if anyone thinks it would be handy.

@flovilmart
Copy link
Contributor

Yep go ahead!

@montymxb
Copy link
Contributor Author

Went up to supporting year, also added d as an option for day. Was bugging me that it's the only that doesn't have an abbreviation...

@montymxb montymxb changed the title Relative Time 'now' Relative Time now,weeks,months,years Oct 30, 2017
@montymxb
Copy link
Contributor Author

Travis CI is having a capacity related issue that seems to have brought quite a few linux builds to a halt. It's locked up a PR in the php sdk as well, and anything else would probably have just about the same luck. This should be clearing up in an hour or two, but it's pending.

@codecov
Copy link

codecov bot commented Oct 31, 2017

Codecov Report

Merging #4304 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4304      +/-   ##
==========================================
- Coverage    92.5%   92.48%   -0.02%     
==========================================
  Files         118      118              
  Lines        8246     8251       +5     
==========================================
+ Hits         7628     7631       +3     
- Misses        618      620       +2
Impacted Files Coverage Δ
src/ParseServer.js 94.35% <ø> (ø) ⬆️
src/Adapters/Storage/Mongo/MongoTransform.js 85.34% <100%> (+0.12%) ⬆️
src/RestWrite.js 93.21% <0%> (-0.19%) ⬇️
...dapters/Storage/Postgres/PostgresStorageAdapter.js 96.5% <0%> (-0.12%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 84aadba...43c1906. Read the comment docs.

@montymxb montymxb requested a review from flovilmart October 31, 2017 05:25
@flovilmart
Copy link
Contributor

@marvelm are you OK with those changes?

@@ -256,6 +256,7 @@ class ParseServer {
/* eslint-disable no-console */
console.warn(`\nWARNING, Unable to connect to '${Parse.serverURL}'.` +
` Cloud code and push notifications may be unavailable!\n`);
/* eslint-enable no-console */
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

return {
status: 'success',
info: 'past',
result: new Date(now.valueOf() - milliseconds)
};
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@marvelm
Copy link
Contributor

marvelm commented Nov 1, 2017

@montymxb Sorry for not raising this issue sooner. With this syntax, I'd expect the feature to keep the day of month constant like Moment.js.

> var moment = require('moment')
> var now = '2017-11-01T11:39:45.657'
> moment(now).add(1, 'month')
moment("2017-12-01T11:39:45.657")
> moment(now).add(2, 'month')
moment("2018-01-01T11:39:45.657")
> moment(now).add(3, 'month')
moment("2018-02-01T11:39:45.657")
> moment(now).add(4, 'month')
moment("2018-03-01T11:39:45.657")
> moment(now).add(5, 'month')
moment("2018-04-01T11:39:45.657")

How the feature is currently implemented:

> moment(now).add(30, 'days')
moment("2017-12-01T11:39:45.657")
> moment(now).add(60, 'days')
moment("2017-12-31T11:39:45.657")
> moment(now).add(90, 'days')
moment("2018-01-30T11:39:45.657")
> moment(now).add(120, 'days')
moment("2018-03-01T11:39:45.657")

I think we should tell users to specify '30 days' explicitly and remove support for month. Otherwise, the code looks good.

@montymxb
Copy link
Contributor Author

montymxb commented Nov 1, 2017

@marvelm that sounds good to me. I'll drop month and push up the changes in a moment.

@montymxb
Copy link
Contributor Author

montymxb commented Nov 1, 2017

Thanks @marvelm !

@montymxb
Copy link
Contributor Author

montymxb commented Nov 1, 2017

@flovilmart any outstanding concerns before we move this in?

@montymxb montymxb merged commit 46af1b6 into parse-community:master Nov 1, 2017
@montymxb montymxb deleted the relative-time-now branch November 1, 2017 20:31
UnderratedDev pushed a commit to UnderratedDev/parse-server that referenced this pull request Mar 21, 2020
* Adds 'now' as an option in relative time

* reenables no-console in previous spot

* Adds weeks,months,years and abbreviations

* modified tests to address coverage

* month be gone!
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.

3 participants