senaduka/old-web
1
0
Fork 0
Code Issues Pull Requests 5 Actions Packages Projects Releases Wiki Activity

Add even more filters #77

Merged
RabbIT09-n merged 5 commits from add-even-more-filters into master 2020-01-31 22:50:55 +01:00
Conversation 35 Commits 5 Files Changed 2 +380 -100
RabbIT09-n commented 2019-12-15 22:34:31 +01:00 (Migrated from gitlab.com)
Copy Link

Changed helpers>db>realEstates.js and searchRequest.js to include more filters (as prepared from crawler and frontend part).

Changed helpers>db>realEstates.js and searchRequest.js to include more filters (as prepared from crawler and frontend part).
bilal.catic commented 2019-12-16 21:52:41 +01:00 (Migrated from gitlab.com)
Copy Link

added 5 commits

  • 5a2fdb72...547411f1 - 4 commits from branch master
  • 73b3f0d2 - Merge branch 'master' into 'add-even-more-filters'

Compare with previous version

added 5 commits <ul><li>5a2fdb72...547411f1 - 4 commits from branch <code>master</code></li><li>73b3f0d2 - Merge branch &#39;master&#39; into &#39;add-even-more-filters&#39;</li></ul> [Compare with previous version](/saburly/marketalarm/web/merge_requests/77/diffs?diff_id=67866584&start_sha=5a2fdb7291b3581320062750cfd769e80f75eb0c)
bilal.catic commented 2019-12-16 22:04:48 +01:00 (Migrated from gitlab.com)
Copy Link

added 1 commit

Compare with previous version

added 1 commit <ul><li>76f4ed0a - apply prettier</li></ul> [Compare with previous version](/saburly/marketalarm/web/merge_requests/77/diffs?diff_id=67869608&start_sha=73b3f0d22ff55437cdcf9973a57d54821ecc3ea2)
bilal.catic commented 2019-12-17 02:05:38 +01:00 (Migrated from gitlab.com)
Copy Link

This "if" condition block can be replaced like this (and you can omit "await") :

return db.RealEstate.findAll({
  where: includeIncompleteAds ? queryIncludeIncomplete : query,
  limit: maxResults,
  order
});
This "if" condition block can be replaced like this (and you can omit "await") : ``` return db.RealEstate.findAll({ where: includeIncompleteAds ? queryIncludeIncomplete : query, limit: maxResults, order }); ```
bilal.catic commented 2019-12-17 09:56:13 +01:00 (Migrated from gitlab.com)
Copy Link

real estates will not have value "ANY" for "accessRoadType" (you can find values like "MACADAM", "ASPHALT",..., or NULL value, so this condition will not work. You can remove "accessRoadType" filter from query in this place and check comment below (where other attributes are checked) for suggestion how to add this filter

real estates will not have value "ANY" for "accessRoadType" (you can find values like "MACADAM", "ASPHALT",..., or NULL value, so this condition will not work. You can remove "accessRoadType" filter from query in this place and check comment below (where other attributes are checked) for suggestion how to add this filter
bilal.catic commented 2019-12-17 09:57:49 +01:00 (Migrated from gitlab.com)
Copy Link

Same comment as for "query"

Same comment as for "query"
bilal.catic commented 2019-12-17 10:00:39 +01:00 (Migrated from gitlab.com)
Copy Link

Here you can check if "accessRoadType" variable is "ANY". If it is, you can ommit "accessRoadType" from query, so it will fetch all road types (even with null values).

If "accessRoadType" is different from "ANY", just add filter for this value (add to the query object).

Also don't forget "queryIncludeIncomplete" object (if you add filter for "accessRoadType" to the query object, also add to the queryIncludeIncomplete)

Here you can check if "accessRoadType" variable is "ANY". If it is, you can ommit "accessRoadType" from query, so it will fetch all road types (even with null values). If "accessRoadType" is different from "ANY", just add filter for this value (add to the query object). Also don't forget "queryIncludeIncomplete" object (if you add filter for "accessRoadType" to the query object, also add to the queryIncludeIncomplete)
RabbIT09-n commented 2019-12-17 10:13:15 +01:00 (Migrated from gitlab.com)
Copy Link

I understood. I ll try to remember to use ternary for next time.

I understood. I ll try to remember to use ternary for next time.
RabbIT09-n commented 2019-12-17 11:09:21 +01:00 (Migrated from gitlab.com)
Copy Link

I changed this (prettier, ternary op and accesRoadType mistake). Should I commit change or wait for more comments-corrections? I don't know what is recommended policy :)

I changed this (prettier, ternary op and accesRoadType mistake). Should I commit change or wait for more comments-corrections? I don't know what is recommended policy :)
RabbIT09-n commented 2019-12-17 11:29:44 +01:00 (Migrated from gitlab.com)
Copy Link

changed this line in version 4 of the diff

changed this line in [version 4 of the diff](/saburly/marketalarm/web/merge_requests/77/diffs?diff_id=67948460&start_sha=76f4ed0a301961346e18484da8418ce3b8650896#63be00c5db0bf65940b389f3d1d05c221c1fb301_290_288)
RabbIT09-n commented 2019-12-17 11:29:44 +01:00 (Migrated from gitlab.com)
Copy Link

changed this line in version 4 of the diff

changed this line in [version 4 of the diff](/saburly/marketalarm/web/merge_requests/77/diffs?diff_id=67948460&start_sha=76f4ed0a301961346e18484da8418ce3b8650896#63be00c5db0bf65940b389f3d1d05c221c1fb301_146_145)
RabbIT09-n commented 2019-12-17 11:29:44 +01:00 (Migrated from gitlab.com)
Copy Link

changed this line in version 4 of the diff

changed this line in [version 4 of the diff](/saburly/marketalarm/web/merge_requests/77/diffs?diff_id=67948460&start_sha=76f4ed0a301961346e18484da8418ce3b8650896#63be00c5db0bf65940b389f3d1d05c221c1fb301_177_170)
RabbIT09-n commented 2019-12-17 11:29:45 +01:00 (Migrated from gitlab.com)
Copy Link

added 2 commits

  • c672b3ab - First review changes: applied prettier, ternary and changed accesRoadType filter
  • 4391aa59 - First review changes: applied prettier, ternary and changed accesRoadType filter

Compare with previous version

added 2 commits <ul><li>c672b3ab - First review changes: applied prettier, ternary and changed accesRoadType filter</li><li>4391aa59 - First review changes: applied prettier, ternary and changed accesRoadType filter</li></ul> [Compare with previous version](/saburly/marketalarm/web/merge_requests/77/diffs?diff_id=67948460&start_sha=76f4ed0a301961346e18484da8418ce3b8650896)
bilal.catic commented 2019-12-17 13:33:41 +01:00 (Migrated from gitlab.com)
Copy Link

Now we have option for users to check if they want to get notified for real estates with some filters undefined, we need to update partial filters (area, gardenSize, numberOfRooms,... ) somehow.

Here is idea but we can discuss more on this :

Let's consider area but this should be reflected on all other partial filters.

Here we have one new real estate with property area and we can have two cases :

  1. area is null (not defined in real estate ad)
  2. area is not null (and is numerical value)

If (1) is TRUE, we should get only those search requests that are interested in real estates with incomplete information.

Else, (2) is TRUE and we can keep current filter to get matching search requests for this real estate.

From this, I would say we can keep current partial filters and then add on the end :

if (!area || !numberOfRooms || !numberOfRooms || ....){
    query.includeIncompleteAds = {
      [Op.eq]: true
    }
}

NOTE : not all partial filters should be included in if condition for all real estate types. For example, if real estate type is LAND, adding !numberOfRooms condition would filter all LAND search requests where includeIncompleteAds is FALSE.

It may sound confusing, so please comment or ask if something is unclear or you have a better approach for this problem :)

Now we have option for users to check if they want to get notified for real estates with some filters undefined, we need to update partial filters (area, gardenSize, numberOfRooms,... ) somehow. Here is idea but we can discuss more on this : Let's consider `area` but this should be reflected on all other partial filters. Here we have one new real estate with property `area` and we can have two cases : 1. `area` is null (not defined in real estate ad) 2. `area` is not null (and is numerical value) If (1) is TRUE, we should get only those search requests that are interested in real estates with incomplete information. Else, (2) is TRUE and we can keep current filter to get matching search requests for this real estate. --------------- From this, I would say we can keep current partial filters and then add on the end : ``` if (!area || !numberOfRooms || !numberOfRooms || ....){ query.includeIncompleteAds = { [Op.eq]: true } } ``` NOTE : not all partial filters should be included in if condition for all real estate types. For example, if real estate type is LAND, adding `!numberOfRooms` condition would filter all LAND search requests where `includeIncompleteAds` is FALSE. ---------------------- It may sound confusing, so please comment or ask if something is unclear or you have a better approach for this problem :)
RabbIT09-n commented 2019-12-18 02:05:17 +01:00 (Migrated from gitlab.com)
Copy Link

added 1 commit

  • 251437f8 - Changed searchRequest to include case of incomplete ads wanted.

Compare with previous version

added 1 commit <ul><li>251437f8 - Changed searchRequest to include case of incomplete ads wanted.</li></ul> [Compare with previous version](/saburly/marketalarm/web/merge_requests/77/diffs?diff_id=68073238&start_sha=4391aa593944dc3b1874cedb0f74eec3bc87b08e)
RabbIT09-n commented 2019-12-18 02:14:49 +01:00 (Migrated from gitlab.com)
Copy Link

I think I understood what is problem with Include Incomplete Ads option. I changed enums.js to include information about which filter should be included based on real estate type. I also changed searchRequest.js in accordance to comments above to consider case that user wants to see incomplete ads (new commit). If I missed something let me know :)

I think I understood what is problem with Include Incomplete Ads option. I changed enums.js to include information about which filter should be included based on real estate type. I also changed searchRequest.js in accordance to comments above to consider case that user wants to see incomplete ads (new commit). If I missed something let me know :)
bilal.catic commented 2019-12-18 07:12:50 +01:00 (Migrated from gitlab.com)
Copy Link

Great, nice work 👍

Great, nice work :+1:
bilal.catic commented 2019-12-18 07:16:14 +01:00 (Migrated from gitlab.com)
Copy Link

we can discuss should we apply this logic on price also or not

we can discuss should we apply this logic on price also or not
bilal.catic commented 2019-12-18 11:25:30 +01:00 (Migrated from gitlab.com)
Copy Link

You can remove else case for price condition, so that we get real estates with or without price

You can remove else case for price condition, so that we get real estates with or without price
bilal.catic commented 2019-12-18 11:42:57 +01:00 (Migrated from gitlab.com)
Copy Link

you can copy price filter from queryIncludeIncomplete to include also real estates without price

you can copy price filter from `queryIncludeIncomplete` to include also real estates without price
RabbIT09-n commented 2019-12-18 21:49:37 +01:00 (Migrated from gitlab.com)
Copy Link

changed this line in version 6 of the diff

changed this line in [version 6 of the diff](/saburly/marketalarm/web/merge_requests/77/diffs?diff_id=68245464&start_sha=251437f815f1dc06bf703ca16d260dff19b16f4c#4008a7e5db195b21917a73b7f4e00687d40aa881_74_74)
RabbIT09-n commented 2019-12-18 21:49:37 +01:00 (Migrated from gitlab.com)
Copy Link

changed this line in version 6 of the diff

changed this line in [version 6 of the diff](/saburly/marketalarm/web/merge_requests/77/diffs?diff_id=68245464&start_sha=251437f815f1dc06bf703ca16d260dff19b16f4c#63be00c5db0bf65940b389f3d1d05c221c1fb301_138_138)
RabbIT09-n commented 2019-12-18 21:49:37 +01:00 (Migrated from gitlab.com)
Copy Link

added 1 commit

  • bee390aa - RealEstate included even is price is null.

Compare with previous version

added 1 commit <ul><li>bee390aa - RealEstate included even is price is null.</li></ul> [Compare with previous version](/saburly/marketalarm/web/merge_requests/77/diffs?diff_id=68245464&start_sha=251437f815f1dc06bf703ca16d260dff19b16f4c)
RabbIT09-n commented 2019-12-18 21:50:20 +01:00 (Migrated from gitlab.com)
Copy Link

Done, now commit!

Done, now commit!
RabbIT09-n commented 2019-12-18 21:50:40 +01:00 (Migrated from gitlab.com)
Copy Link

Also done, new commit!

Also done, new commit!
bilal.catic commented 2019-12-18 22:19:13 +01:00 (Migrated from gitlab.com)
Copy Link

resolved all threads

resolved all threads
RabbIT09-n commented 2019-12-21 02:21:15 +01:00 (Migrated from gitlab.com)
Copy Link

changed this line in version 7 of the diff

changed this line in [version 7 of the diff](/saburly/marketalarm/web/merge_requests/77/diffs?diff_id=68595168&start_sha=bee390aa1572d764784a19425dc7352872fa1b5a#63be00c5db0bf65940b389f3d1d05c221c1fb301_178_178)
RabbIT09-n commented 2019-12-21 02:21:15 +01:00 (Migrated from gitlab.com)
Copy Link

changed this line in version 7 of the diff

changed this line in [version 7 of the diff](/saburly/marketalarm/web/merge_requests/77/diffs?diff_id=68595168&start_sha=bee390aa1572d764784a19425dc7352872fa1b5a#4008a7e5db195b21917a73b7f4e00687d40aa881_76_76)
RabbIT09-n commented 2019-12-21 02:21:15 +01:00 (Migrated from gitlab.com)
Copy Link

added 1 commit

  • 42ff1f76 - Changed to avoid falsy values and not defined realestate parametrs.

Compare with previous version

added 1 commit <ul><li>42ff1f76 - Changed to avoid falsy values and not defined realestate parametrs.</li></ul> [Compare with previous version](/saburly/marketalarm/web/merge_requests/77/diffs?diff_id=68595168&start_sha=bee390aa1572d764784a19425dc7352872fa1b5a)
RabbIT09-n commented 2019-12-26 23:30:43 +01:00 (Migrated from gitlab.com)
Copy Link

added 1 commit

Compare with previous version

added 1 commit <ul><li>d5d3a1f3 - Changed accesRoadType logic</li></ul> [Compare with previous version](/saburly/marketalarm/web/merge_requests/77/diffs?diff_id=68977634&start_sha=42ff1f762ff3a5f0966ea783e8a90d6f2b823e4c)
RabbIT09-n commented 2020-01-17 01:55:52 +01:00 (Migrated from gitlab.com)
Copy Link

added 11 commits

  • d5d3a1f3...25979914 - 9 commits from branch master
  • 4fd4018b - Merge branch 'sliders-formating' of gitlab.com:saburly/marketalarm/web into add-even-more-filters
  • 870b71a3 - WIP Changed all logic for searchRequest.

Compare with previous version

added 11 commits <ul><li>d5d3a1f3...25979914 - 9 commits from branch <code>master</code></li><li>4fd4018b - Merge branch &#39;sliders-formating&#39; of gitlab.com:saburly/marketalarm/web into add-even-more-filters</li><li>870b71a3 - WIP Changed all logic for searchRequest.</li></ul> [Compare with previous version](/saburly/marketalarm/web/merge_requests/77/diffs?diff_id=71262995&start_sha=d5d3a1f306fdfca8238db556937113d07d7ec13f)
RabbIT09-n commented 2020-01-17 01:55:53 +01:00 (Migrated from gitlab.com)
Copy Link

marked as a Work In Progress from 870b71a3c7

marked as a **Work In Progress** from 870b71a3c712d4ed30b5ec9a3cf85436d7456281
RabbIT09-n commented 2020-01-17 22:59:11 +01:00 (Migrated from gitlab.com)
Copy Link

added 1 commit

  • d1173838 - Tested both ways for realestate and search req filters.

Compare with previous version

added 1 commit <ul><li>d1173838 - Tested both ways for realestate and search req filters.</li></ul> [Compare with previous version](/saburly/marketalarm/web/merge_requests/77/diffs?diff_id=71496439&start_sha=870b71a3c712d4ed30b5ec9a3cf85436d7456281)
RabbIT09-n commented 2020-01-31 22:50:41 +01:00 (Migrated from gitlab.com)
Copy Link

unmarked as a Work In Progress

unmarked as a **Work In Progress**
RabbIT09-n commented 2020-01-31 22:50:55 +01:00 (Migrated from gitlab.com)
Copy Link

merged

merged
RabbIT09-n commented 2020-01-31 22:50:55 +01:00 (Migrated from gitlab.com)
Copy Link

mentioned in commit 4a6bcf262e

mentioned in commit 4a6bcf262ebcfa7c1c84b65eccf2418e6611617a
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
senaduka
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: senaduka/old-web#77
Delete Branch "add-even-more-filters"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?