Add new tests for models #610

kaustubh-nair · 2019-05-15T18:42:39Z

Part of #594

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

PR is descriptively titled 📑 and links the original issue above 🔗
tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
code is in uniquely-named feature branch and has no merge conflicts 📁
screenshots/GIFs are attached 📎 in case of UI updation
ask @publiclab/reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

app/models/map.rb

test/unit/map_test.rb

test/unit/warpable_test.rb

codeclimate · 2019-05-15T18:43:32Z

Code Climate has analyzed commit e0c305b and detected 6 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Style	5
Complexity	1

View more on Code Climate.

kaustubh-nair · 2019-05-15T18:43:48Z

app/models/map.rb

-      end
-      filenames
-    end
-  end


I've removed this method since it is not being used anywhere else.
Here is the relevent commit aadd8b5
I couldn't find public_filenames being called in the current code.

kaustubh-nair · 2019-05-15T18:44:50Z

app/models/map.rb


  def validate
    self.name != 'untitled'
    self.lat >= -90 && self.lat <= 90 && self.lon >= -180 && self.lat <= 180
  end

-  # Hash the password before saving the record
+  #Hash the password before saving the record
  def before_create
    self.password = Password::update(self.password) if self.password != ""
  end


This is supposed to Hash the password but before_create is never called????
Map passwords are stored in plain text???
Is this okay???

Password needs to be encrypted with one way hash functions. This should be problem as per theory

@jywarren Map passwords are not being hashed. They're stored in plain text.
Security concern?

This is quite an old feature, but the problem is that we may have existing maps that use it. Can we confirm that there is no way to /make/ a new passworded map?

app/models/user.rb

kaustubh-nair · 2019-05-15T18:46:43Z

app/models/way.rb

-  def bbox=(bbox)
-    # counting from left, counter-clockwise
-    self.lon1,self.lat2,self.lon2,self.lat1 = bbox
-  end
 end


This method is redundant too.
What is the Way model supposed to do anyway?
It is not being used anywhere

I think it may be used in annotations, but am not sure! Can you look at the annotations interface which may be a bit broken?

But we could address this in another issue later. You could also look at the commits around that time using the history or blame view - see? Yes, this is VERY old! And, i think, can go!

https://github.com/publiclab/mapknitter/blame/e0c305b89a53828006f20de422da2bbffb53a21b/app/models/way.rb

codecov · 2019-05-15T18:49:27Z

Codecov Report

Merging #610 into main will increase coverage by 0.25%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##            main     #610      +/-   ##
=========================================
+ Coverage   41.1%   41.36%   +0.25%     
=========================================
  Files         34       34              
  Lines       1321     1308      -13     
=========================================
- Hits         543      541       -2     
+ Misses       778      767      -11

Impacted Files	Coverage Δ
app/models/user.rb	`75% <ø> (+6.25%)`	⬆️
app/models/map.rb	`39% <100%> (+1.67%)`	⬆️

kaustubh-nair · 2019-05-15T18:54:10Z

@jywarren @sashadev-sky @SidharthBansal @cesswairimu @alaxalves
I've removed some redundant code which was not being used anywhere.
Please check them out.
Coverage for models is at 95% now 🎈 🎈

SidharthBansal · 2019-05-15T19:00:54Z

app/models/map.rb


  def validate
    self.name != 'untitled'
    self.lat >= -90 && self.lat <= 90 && self.lon >= -180 && self.lat <= 180
  end

-  # Hash the password before saving the record
+  #Hash the password before saving the record
  def before_create
    self.password = Password::update(self.password) if self.password != ""
  end


Password needs to be encrypted with one way hash functions. This should be problem as per theory

app/models/user.rb

test/unit/map_test.rb

cesswairimu · 2019-05-15T19:17:57Z

Awesome!

jywarren · 2019-05-22T00:16:08Z

Yes, I think so. But gosh this is old. Do we even allow people to make passworded maps anymore?

…

On Tue, May 21, 2019 at 3:05 AM Kaustubh Nair ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In app/models/map.rb <#610 (comment)>: > def validate self.name != 'untitled' self.lat >= -90 && self.lat <= 90 && self.lon >= -180 && self.lat <= 180 end - # Hash the password before saving the record + #Hash the password before saving the record def before_create self.password = Password::update(self.password) if self.password != "" end @jywarren <https://github.com/jywarren> Map passwords are not being hashed. They're stored in plain text. Security concern? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#610?email_source=notifications&email_token=AAAF6J2Q2U5NY3JAZQS3JPTPWONKPA5CNFSM4HNF6WY2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBZF64KI#discussion_r285876974>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAF6JYGAL4MJDHLDVFK4RLPWONKPANCNFSM4HNF6WYQ> .

kaustubh-nair · 2019-05-27T15:13:19Z

Yes, I think so. But gosh this is old. Do we even allow people to make passworded maps anymore?

Oh yeah, I did not notice that.
Let's not change any password code here and create a new issue to discuss this. ( #641)

kaustubh-nair · 2019-05-27T15:14:29Z

Also, @publiclab/mapknitter-reviewers @jywarren this is ready for merge!

alaxalves · 2019-05-31T14:46:34Z

.rubocop.yml

@@ -0,0 +1,2 @@
+Metrics/BlockLength:


Same as https://github.com/publiclab/mapknitter/pull/615/files#r289414686

alaxalves · 2019-05-31T14:48:52Z

test/unit/annotation_test.rb

@@ -17,6 +17,16 @@ def setup
  end

  test 'geometry type' do


Same as https://github.com/publiclab/mapknitter/pull/615/files#r289417232

Should I wait for these last couple things before merging? Thanks!!!!

alaxalves · 2019-05-31T15:12:57Z

BTW @kaustubh-nair I could work on this one and you could work on #615. What do you think? There are only small patches left for both. Let's get those merged today 💪 💪 💪 💪

kaustubh-nair · 2019-05-31T15:15:54Z

BTW @kaustubh-nair I could work on this one and you could work on #615. What do you think? There are only small patched left for both. Let's get those merged today muscle muscle muscle muscle

@alaxalves Awesome go ahead
Thanks!

alaxalves · 2019-06-01T01:53:28Z

BTW @kaustubh-nair I could work on this one and you could work on #615. What do you think? There are only small patched left for both. Let's get those merged today muscle muscle muscle muscle

@alaxalves Awesome go ahead
Thanks!

Done! @jywarren This is ready to merge

jywarren · 2019-06-03T14:53:15Z

Hi! I believe this just needs a rebase - thank you!!!!

jywarren · 2019-06-03T16:44:05Z

Perfect, thank you!!!!

* Add annotation tests and remove redundant code * Add export tests * Remove some code * Fix codeclimate issues * Fix codeclimate issues * Fix rubocop block length * Removing rubocop cfg since publiclab#547 and renaming tests

kaustubh-nair added 3 commits May 15, 2019 23:13

Add annotation tests and remove redundant code

cfafdcf

Add export tests

162ef37

Remove some code

e0c305b

codeclimate bot reviewed May 15, 2019

View reviewed changes

app/models/map.rb Outdated Show resolved Hide resolved

test/unit/map_test.rb Outdated Show resolved Hide resolved

test/unit/map_test.rb Outdated Show resolved Hide resolved

test/unit/warpable_test.rb Outdated Show resolved Hide resolved

kaustubh-nair commented May 15, 2019

View reviewed changes

app/models/user.rb Show resolved Hide resolved

kaustubh-nair commented May 15, 2019

View reviewed changes

SidharthBansal requested changes May 15, 2019

View reviewed changes

Fix codeclimate issues

f2510de

Fix codeclimate issues

ca68997

Fix rubocop block length

0ca6dec

kaustubh-nair requested review from SidharthBansal and jywarren May 27, 2019 15:15

kaustubh-nair closed this May 27, 2019

kaustubh-nair reopened this May 27, 2019

Merge branch 'main' into model-tests

5d14d23

alaxalves reviewed May 31, 2019

View reviewed changes

alaxalves force-pushed the model-tests branch 2 times, most recently from 75fa2f1 to 2507f16 Compare June 1, 2019 02:13

Removing rubocop cfg since #547 and renaming tests

0c69df6

alaxalves force-pushed the model-tests branch from 2507f16 to 0c69df6 Compare June 1, 2019 02:20

kaustubh-nair added 2 commits June 3, 2019 20:43

Fix conflicts

1d78bb4

Fix conflicts

cc84b28

kaustubh-nair added the ready label Jun 3, 2019

jywarren merged commit 03deef1 into main Jun 3, 2019

alaxalves deleted the model-tests branch June 3, 2019 19:12

kaustubh-nair mentioned this pull request Jun 9, 2019

Weekly Community Check-In #24 (Learning from Public Lab) publiclab/plots2#5828

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new tests for models #610

Add new tests for models #610

kaustubh-nair commented May 15, 2019

codeclimate bot commented May 15, 2019

kaustubh-nair May 15, 2019

kaustubh-nair May 15, 2019

SidharthBansal May 15, 2019

kaustubh-nair May 21, 2019

jywarren May 28, 2019

kaustubh-nair May 15, 2019 •

edited

Loading

jywarren May 28, 2019

jywarren May 28, 2019

codecov bot commented May 15, 2019 •

edited

Loading

kaustubh-nair commented May 15, 2019

SidharthBansal May 15, 2019

cesswairimu commented May 15, 2019

jywarren commented May 22, 2019 via email

kaustubh-nair commented May 27, 2019 •

edited

Loading

kaustubh-nair commented May 27, 2019 •

edited

Loading

alaxalves May 31, 2019

alaxalves May 31, 2019

jywarren May 31, 2019

alaxalves commented May 31, 2019 •

edited

Loading

kaustubh-nair commented May 31, 2019

alaxalves commented Jun 1, 2019

jywarren commented Jun 3, 2019

jywarren commented Jun 3, 2019

Add new tests for models #610

Add new tests for models #610

Conversation

kaustubh-nair commented May 15, 2019

codeclimate bot commented May 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kaustubh-nair May 15, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented May 15, 2019 • edited Loading

Codecov Report

kaustubh-nair commented May 15, 2019

Choose a reason for hiding this comment

cesswairimu commented May 15, 2019

jywarren commented May 22, 2019 via email

kaustubh-nair commented May 27, 2019 • edited Loading

kaustubh-nair commented May 27, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alaxalves commented May 31, 2019 • edited Loading

kaustubh-nair commented May 31, 2019

alaxalves commented Jun 1, 2019

jywarren commented Jun 3, 2019

jywarren commented Jun 3, 2019

kaustubh-nair May 15, 2019 •

edited

Loading

codecov bot commented May 15, 2019 •

edited

Loading

kaustubh-nair commented May 27, 2019 •

edited

Loading

kaustubh-nair commented May 27, 2019 •

edited

Loading

alaxalves commented May 31, 2019 •

edited

Loading